Fix use with Bazel #55
Conversation
src/lib.rs
Outdated
|
|
||
| fn workspace_manifest_path(cargo_toml_manifest: &Path) -> Result<Option<PathBuf>, Error> { | ||
| // Skip invoking Cargo if we're building a crate packaged with cargo. | ||
| if !cargo_toml_manifest.with_extension(".toml.orig").exists() { |
There was a problem hiding this comment.
| if !cargo_toml_manifest.with_extension(".toml.orig").exists() { | |
| if cargo_toml_manifest.with_extension(".toml.orig").exists() { |
Isn't that the correct way?
There was a problem hiding this comment.
Because this file exists if the package was packaged by cargo, but also generally a little bit fragile because anything could create this file.
Maybe to fix your issue, just returning None when CARGO env variable wasn't found?
There was a problem hiding this comment.
Ooops, patch tested and submitted differ :( my bad, sorry.
Yes, I’d be happy to change the PR to return None of cargo isn’t set. I guess the corresponding error type can also go away then?
There was a problem hiding this comment.
To clarify: intended was this:
if cargo_toml_manifest.with_extension("toml.orig").exists() {(note the removed leading dot)
There was a problem hiding this comment.
Pushed the version that I intended and that I verified that it works as intended.
I think it's a win-win kind of, as it also speeds up the regular builds. But if you prefer the None on unset Cargo env, then I can also do that.
There was a problem hiding this comment.
I'm a little bit afraid it breaks something else :D Also in a non obvious way. I think for now I would opt-in to check for CARGO being unchecked. But we could open an issue to speed up regular builds.
There was a problem hiding this comment.
Alright, sounds good. Force-push coming up to correct this :)
When the application is built with Bazel as build system, environment variables like CARGO_MANIFEST_DIR, etc. are set for compatibility, but CARGO itself isn't, because Bazel is the tool of choice. Therefore any attempt to invoke Cargo to locate the workspace manifest path fails. As a fallback, a lack of the CARGO environment variable now just means no workspace support.
| NotFound(PathBuf), | ||
| CargoManifestDirNotSet, | ||
| CargoEnvVariableNotSet, | ||
| FailedGettingWorkspaceManifestPath, |
There was a problem hiding this comment.
| FailedGettingWorkspaceManifestPath, | |
| #[deprecated] | |
| CargoEnvVariableNotSet, | |
| FailedGettingWorkspaceManifestPath, |
| path.display(), | ||
| ), | ||
| Error::CargoEnvVariableNotSet => f.write_str("`CARGO` env variable not set."), | ||
| Error::FailedGettingWorkspaceManifestPath => |
There was a problem hiding this comment.
| Error::FailedGettingWorkspaceManifestPath => | |
| #[allow(deprecated)] | |
| Error::CargoEnvVariableNotSet => f.write_str("`CARGO` env variable not set."), | |
| Error::FailedGettingWorkspaceManifestPath => |
|
Ty! |
rstest currently fails to build under bazel with
```
error: custom attribute panicked
|
250 | #[rstest]
| ^^^^^^^^^
|
= help: message: rstest is present in `Cargo.toml` qed: Could not find `Cargo.toml` in manifest dir:
```
I've tracked this back to proc-macro-crate, which fixed it in bkchr/proc-macro-crate#55
rstest currently fails to build under bazel with
```
error: custom attribute panicked
|
250 | #[rstest]
| ^^^^^^^^^
|
= help: message: rstest is present in `Cargo.toml` qed: Could not find `Cargo.toml` in manifest dir:
```
I've tracked this back to proc-macro-crate, which fixed it in bkchr/proc-macro-crate#55
rstest currently fails to build under bazel with
```
error: custom attribute panicked
|
250 | #[rstest]
| ^^^^^^^^^
|
= help: message: rstest is present in `Cargo.toml` qed: Could not find `Cargo.toml` in manifest dir:
```
I've tracked this back to proc-macro-crate, which fixed it in bkchr/proc-macro-crate#55
rstest currently fails to build under bazel with
```
error: custom attribute panicked
|
250 | #[rstest]
| ^^^^^^^^^
|
= help: message: rstest is present in `Cargo.toml` qed: Could not find `Cargo.toml` in manifest dir:
```
I've tracked this back to proc-macro-crate, which fixed it in bkchr/proc-macro-crate#55
#329) * Upgrade proc-macro-crate to 3.4.0 in order fix compilation under bazel rstest currently fails to build under bazel with ``` error: custom attribute panicked | 250 | #[rstest] | ^^^^^^^^^ | = help: message: rstest is present in `Cargo.toml` qed: Could not find `Cargo.toml` in manifest dir: ``` I've tracked this back to proc-macro-crate, which fixed it in bkchr/proc-macro-crate#55 * fix typo in changelog
2 participants
When the application is built with Bazel as build system, environment
variables like CARGO_MANIFEST_DIR, etc. are set for compatibility, but
CARGO itself isn't, because Bazel is the tool of choice. Therefore any
attempt to invoke Cargo to locate the workspace manifest path fails.
As a fallback, a lack of the CARGO environment variable now just means
no workspace support.