Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct dependencies and build checks #484

Merged
merged 9 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ jobs:
release-arch: amd64
env:
RUST_BACKTRACE: full
# Force not building debuginfo to save space on disk.
RUSTFLAGS: "-C debuginfo=0"
RUSTC_WRAPPER: sccache
RUSTV: ${{ matrix.rust }}
SCCACHE_CACHE_SIZE: 2G
Expand Down Expand Up @@ -135,7 +137,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: check
args: --all --bins --tests --examples
args: --workspace --all-features --bins --tests --examples --benches
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why having --all-features is good to test against, we also need to have tests (or at least build checks) for each individual feature, to make sure they work by themselves and with no features enabled at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true. I was trying to take small steps.

Given how difficult it was to get this to be happy in CI I'd prefer to do that as a followup since it already fixes some things on main and would stop regressions to that instead of having to keep playing catchup with main while working on this - as already happened.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was not meant to add more work to this pr, just a reminder for all of us that it needs more work 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


- name: tests
uses: actions-rs/cargo@v1
Expand All @@ -145,17 +147,17 @@ jobs:
# GHA if this is not present
# https://twitter.com/steipete/status/921066430220652544?lang=en
# https://blog.phusion.nl/2017/10/13/why-ruby-app-servers-break-on-macos-high-sierra-and-what-can-be-done-about-it/
OBJC_DISABLE_INITIALIZE_FORK_SAFETY: YES
OBJC_DISABLE_INITIALIZE_FORK_SAFETY: "YES"
with:
command: test
args: --all -j 4
args: -j 2 --workspace --all-features --examples --benches --bins

- name: clipy
- name: clippy
uses: actions-rs/cargo@v1
if: matrix.os == 'ubuntu-latest' && matrix.rust=='stable'
with:
command: clippy
args: --all --tests --benches --all-targets -- -D warnings
command: clippy
args: --workspace --all-features --tests --benches --examples --bins --all-targets -- -D warnings

- name: build release
uses: actions-rs/cargo@v1
Expand Down Expand Up @@ -257,4 +259,4 @@ jobs:
- uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check
args: --all -- --check
6 changes: 3 additions & 3 deletions DEVELOPERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ When you push your branch to github and open up a pull request, it will automati
In order to catch linting and testing errors before pushing the code to github, be sure to run:

```shell
$ cargo clippy --workspace --examples --benches --tests
$ cargo test --workspace --examples --benches
$ cargo clippy --workspace --all-features --examples --benches --tests --bins
$ cargo test --workspace --all-features --examples --benches --bins
```

Setting up a [git hook][git-hook] to run these commands can save you many headaches:

```shell
#!/bin/sh
cargo clippy --workspace --examples --tests --benches && cargo test --workspace --examples
cargo clippy --workspace --all-features --examples --benches --tests --bins && cargo test --workspace --all-features --examples --benches --bins
```

## <a name="dependecies"></a> Dependencies
Expand Down
2 changes: 1 addition & 1 deletion iroh-metrics/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ mod tests {
#[cfg(feature = "tokio-console")]
expect.insert(
"tokio_console".to_string(),
Value::new(None, cfg.tokio_console.clone()),
Value::new(None, cfg.tokio_console),
);
let got = cfg.collect().unwrap();
for key in got.keys() {
Expand Down
3 changes: 2 additions & 1 deletion iroh-one/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ iroh-store = {path = "../iroh-store", default-features = false, features = ["rpc
iroh-util = {path = "../iroh-util"}
reqwest = {version = "0.11", features = ["rustls-tls"], default-features = false}
serde = {version = "1.0", features = ["derive"]}
tempfile = { version = "3.3.0", optional = true }
tokio = {version = "1", features = ["macros", "rt-multi-thread", "process"]}
tracing = "0.1.33"

Expand All @@ -41,4 +42,4 @@ rpc-grpc = ["iroh-rpc-types/grpc", "iroh-rpc-client/grpc", "iroh-metrics/rpc-grp
rpc-mem = ["iroh-rpc-types/mem", "iroh-rpc-client/mem"]
# uds-gateway controls whether http over uds is enabled. This is independent of the
# rpc-grpc control endpoint.
uds-gateway = []
uds-gateway = ["tempfile"]
11 changes: 9 additions & 2 deletions iroh-one/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl Config {
/// as a single entry point for other system services if feature enabled.
pub fn default_rpc_config() -> RpcClientConfig {
#[cfg(feature = "uds-gateway")]
let path: PathBuf = tempdir::TempDir::new("iroh")
let path: PathBuf = tempfile::Builder::new()
.prefix("iroh")
.tempfile()
.unwrap()
.path()
.join("ipfsd.http");
Expand Down Expand Up @@ -93,7 +95,12 @@ impl Config {
impl Default for Config {
fn default() -> Self {
#[cfg(feature = "uds-gateway")]
let gateway_uds_path: PathBuf = TempDir::new("iroh").unwrap().path().join("ipfsd.http");
let gateway_uds_path: PathBuf = tempfile::Builder::new()
.prefix("iroh")
.tempfile()
.unwrap()
.path()
.join("ipfsd.http");
let rpc_client = Self::default_rpc_config();
let metrics_config = MetricsConfig::default();
let store_config =
Expand Down
10 changes: 6 additions & 4 deletions iroh-one/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ use iroh_rpc_client::Client as RpcClient;
use iroh_rpc_types::Addr;
use iroh_util::lock::ProgramLock;
use iroh_util::{iroh_config_path, make_config};
#[cfg(feature = "uds-gateway")]
use tempdir::TempDir;
use tokio::sync::RwLock;

#[tokio::main(flavor = "multi_thread")]
Expand Down Expand Up @@ -109,14 +107,18 @@ async fn main() -> Result<()> {

#[cfg(feature = "uds-gateway")]
let uds_server_task = {
let mut path = TempDir::new("iroh")?.path().join("ipfsd.http");
let mut path = tempfile::Builder::new()
.prefix("iroh")
.tempdir()?
.path()
.join("ipfsd.http");
if let Some(uds_path) = config.gateway_uds_path {
path = uds_path;
} else {
// Create the parent path when using the default value since it's likely
// it won't exist yet.
if let Some(parent) = path.parent() {
let _ = std::fs::create_dir_all(&parent);
let _ = std::fs::create_dir_all(parent);
}
}

Expand Down
2 changes: 1 addition & 1 deletion iroh-one/src/uds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use hyper::{
server::accept::Accept,
};
use iroh_gateway::{core::State, handlers::get_app_routes};
use iroh_resolver::resolver::ContentLoader;
use iroh_resolver::content_loader::ContentLoader;
use std::path::PathBuf;
use std::{
io,
Expand Down
2 changes: 1 addition & 1 deletion iroh-share/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ libp2p = { version = "0.50", default-features = false, features = ["gossipsub"]
multibase = "0.9.1"
rand = "0.8.5"
serde = { version = "1", features = ["derive"] }
tempfile = "3.3"
tempfile = "3.3.0"
tokio = { version = "1" }
tokio-stream = "0.1.9"
tracing = "0.1.34"
Expand Down