-
Notifications
You must be signed in to change notification settings - Fork 156
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
Workspace configuration of flux crate confuses rustc #2342
Comments
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 23, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
to rockstar/flux
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes influxdata#2342
rockstar
added a commit
that referenced
this issue
Nov 24, 2020
_Obligatory: apologies for using the term "idiomatic" here; I couldn't find a better word._ When working with a rust workspace, it is usually in the context of multiple connected crates. Our use of multiple crates is needed and makes sense. However, the crates aren't laid out the way a normal crate would be laid out, and the `src` in the workspace is usually not there. This patch simplifies the rust workspace layout, removes some overrides in various `Cargo.toml` files, and fixes the build accordingly. Fixes #2342
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Preface: I need to unpack some things I've noticed about the workspace configuration of libflux. A lot of this is just guessing, but I'm starting to see a pattern.
Right now,
libflux
is a cargo workspace, meaning it's a collection of crates. Inside of that workspace, we have theflux
crate. Through just the evolutionary process, that crate source is found atlibflux/src/flux
, whereas a normal crate-in-workspace would be inlibflux/flux/src
. There are some configurations in the workspace to support this non-standard sort of approach, so in general, it should be fine.Unfortunately, it seems that it confuses rustc's ability to figure out what needs to be compiled. For instance, if I
cargo clippy
, change a file, andcargo clippy
again,rustc
signals that there are no changes. What's odd is that withcargo build
, change a file, andcargo build
again,rustc
determines that everything needs to be rebuilt[1].It's possible (and in the case of
cargo build
, highly likely) that there are other moving parts at play, but this is not an issue I expect to see in a workspace using default configuration. I suggest making theflux
crate more idiomatic, swapping where thesrc
folder lives. At that point, we eliminate a non-common codepath, which makes it easier to track down these issues.[1] One of the other potential issues with build times is related to external build tools and the complexities of
build.rs
. That's another moving piece that may need to be addressed.The text was updated successfully, but these errors were encountered: