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

Cannot find workspace version in crate_package #23

Open
jonathanbourdier opened this issue Feb 12, 2024 · 11 comments
Open

Cannot find workspace version in crate_package #23

jonathanbourdier opened this issue Feb 12, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@jonathanbourdier
Copy link

When I use cargo workspace, version is pulled from the crate's cargo.toml instead of workspace cargo.toml, resulting in a bug.

It should work with this cargo.toml :

[package]
name = "my_package"
version.workspace = true
authors.workspace = true
edition.workspace = true

lib.rs line 359: pub fn crate_package(&self) -> Result

This lib is used by i18n_embed which causes the impossibility to use it in a workspace.

@taiki-e taiki-e added the bug Something isn't working label Feb 22, 2024
@Umio-Yasuno
Copy link

This issue can be fixed with the following patch.
However, it is debatable whether it is correct to return a string like "*".

diff --git a/src/lib.rs b/src/lib.rs
index 8f28131..2f1735d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -371,9 +371,15 @@ impl Manifest {
             Error::InvalidManifest("[package] section is missing `version`".to_string())
         })?;
 
-        let package_version = package_version_value.as_str().ok_or_else(|| {
-            Error::InvalidManifest("`version` in [package] section is not a string".to_string())
-        })?;
+        let package_version = match package_version_value {
+            Value::String(s) => s.clone(),
+            Value::Table(t) => "*".to_string(),
+            _ => {
+                return Err(Error::InvalidManifest(
+                    "`version` in [package] section is not a string".to_string(),
+                ))
+            }
+        };
 
         let package = Package {
             key: package_key.to_string(),

@taiki-e
Copy link
Owner

taiki-e commented Apr 17, 2024

I think the correct approach is to return the version defined in the [workspace.dependencies].

@Umio-Yasuno
Copy link

I think the correct approach is to return the version defined in the [workspace.dependencies].

Yeah, we need a stable way to get the workspace root.
rust-lang/cargo#3946

@Umio-Yasuno
Copy link

Could std::env::var_os("CARGO_PKG_VERSION") be used instead?

@taiki-e
Copy link
Owner

taiki-e commented Apr 19, 2024

Could std::env::var_os("CARGO_PKG_VERSION") be used instead?

CARGO_PKG_VERSION is the version of the current package, not dependency.

@taiki-e
Copy link
Owner

taiki-e commented Apr 19, 2024

Yeah, we need a stable way to get the workspace root.
rust-lang/cargo#3946

Given this limitation, it may actually be a reasonable compromise to return * for now, with documentation that the exact version requirements are not returned for workspace dependencies.

@Umio-Yasuno
Copy link

CARGO_PKG_VERSION is the version of the current package, not dependency.

I tested it and found that it only helps with this issue when using the proc macro like cargo-i18n ...

@taiki-e
Copy link
Owner

taiki-e commented Apr 20, 2024

CARGO_PKG_VERSION

If it's about the current package version and not the dependency version, that could indeed still work well in some cases.
(I thought this issue was about workspace dependencies.)

@taiki-e
Copy link
Owner

taiki-e commented Apr 20, 2024

Wait, why did we have crate_package in the first place? Did env!("CARGO_PKG_VERSION") not work?

cc @kellpossible (because you opened #11)

@Umio-Yasuno
Copy link

Umio-Yasuno commented Apr 20, 2024

I'm proposing at cargo-i18n to fallback to std::env::var("CARGO_PKG_NAME") .

kellpossible/cargo-i18n#97 (comment)

@Umio-Yasuno
Copy link

Wait, why did we have crate_package in the first place? Did env!("CARGO_PKG_VERSION") not work?

In the case of env!, the value from the crate (i18n-embed-fl) is used instead of the package being compiled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants