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 compiler errors: lazy_static - Specify version 1.4.0 in Cargo.lock #5721

Closed
wants to merge 1 commit into from

Conversation

SirMangler
Copy link

Following up on issue #5702 in regards to lazy_static failing to compile with newer versions of rust. The issue has been reported in the upstream however the issue hasn't been acknowledged in 4+ months, so it seems clear we can't expect upstream to swiftly resolve this for us. An ideal long term solution would be to phase out lazy_static, but many crates continue to use this crate, meaning we would be required to either phase out those crates as well or wait for them to resolve the issue upstream. Both will take a lot of time.

I have determined that version lazy_static 1.4.0 seems to work on the latest stable version of rust (1.80.0 as of this pr). As a temporary but potentially indefinite solution, I propose re-adding the Cargo.lock file to exclusively include lazy_static 1.4.0. This way, the Cargo.lock file will still be generated per-build for all other crates, but the relevant dependencies will be made to use the version of lazy_static which I have determined to work, which resolves the issue of slint failing to compile.

The only potential consequence I can see from this change is that any new crates that may potentially be added to slint which for some reason would require lazy_static 1.5.0 will reintroduce this compiler error as the Cargo.lock file could automatically update to the newer version. This might not be clear to the developer at the time if they are working on an older Rust version that does not have any issues with lazy_static 1.5.0. However I believe the risk of this to be minimal, and as a temporary solution it's worthwhile until a more permanent solution can be put in place.

@CLAassistant
Copy link

CLAassistant commented Jul 30, 2024

CLA assistant check
All committers have signed the CLA.

@tronical
Copy link
Member

I'd prefer not to have a Cargo.lock in the repo, except for release tags. We have a need for this also occasionally due to the MSRV. We could do the same play here: In CI builds downgrade lazy static to 1.4, and for releases create a Cargo.lock with lazy_static downgraded.

In practice this only helps C++ users who build from git tags, as well as Python/JavaScript. Rust developers who get slint from crates.io will still see the problem.

I still wonder why we don't see this issue in the CI and what exactly triggers it (I also don't see it locally).

@SirMangler
Copy link
Author

I still wonder why we don't see this issue in the CI and what exactly triggers it (I also don't see it locally).

I'm curious, what rust version are you on?

@SirMangler
Copy link
Author

In practice this only helps C++ users who build from git tags, as well as Python/JavaScript. Rust developers who get slint from crates.io will still see the problem.

Unfortunately you're right, I was focused on testing the examples and lost sight of how slint is generally used by rust developers. In that case should I change this PR to be for the release tag for C++ developers? Otherwise feel free to close this PR.

@ogoffart
Copy link
Member

Thanks for this PR.
Adding a lockfile on the repository wouldn't fix this problem for our users, and would prevent our CI to detect issues with newer version of the different crates.
The problem seems to be more complex than simply the version number.
So i'm going to close this PR
Note that I appreciate your effort to help us fixing this issue.
We can continue the discussion in #5702

@ogoffart ogoffart closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants