Skip to content

WIP: Split crates requiring nightly Rust into separate unstable workspace #1227

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshtriplett
Copy link
Contributor

@joshtriplett joshtriplett commented May 7, 2025

No description provided.

@joshtriplett joshtriplett changed the title Temporarily exclude crates that require nightly WIP: Temporarily exclude crates that require nightly May 7, 2025
@joshtriplett
Copy link
Contributor Author

Marking as WIP, because it'll need some CI debugging to make sure tests don't fail.

@kkysen
Copy link
Contributor

kkysen commented May 8, 2025

Also I merged #1225 so you can rebase now.

@joshtriplett
Copy link
Contributor Author

Currently attempting a full implementation of #1220. Still WIP; will work on ensuring CI passes.

@joshtriplett joshtriplett changed the title WIP: Temporarily exclude crates that require nightly WIP: Split crates requiring nightly Rust into separate unstable workspace May 8, 2025
@joshtriplett
Copy link
Contributor Author

Side note: analysis/tests/minimal/reference_pdg.bc was checked in despite being .gitignored. It doesn't look like that test is using "compare" mode, so I don't think that file is being used, only written to. Nonetheless, I moved it across to unstable/analysis/tests/minimal/reference_pdg.bc. If there's any potential cleanup there, it can happen separately from this PR.

@joshtriplett joshtriplett force-pushed the try-stable branch 5 times, most recently from bb14e51 to 444ca79 Compare May 8, 2025 15:13
@joshtriplett
Copy link
Contributor Author

joshtriplett commented May 8, 2025

@kkysen For some reason, the Azure pipeline doesn't seem to be kicking off for pushes anymore, and I'm not sure why. I think I've fixed most of the issues at this point, but I can't test that until the Azure pipeline runs again.

EDIT: Seems to be working again.

@joshtriplett
Copy link
Contributor Author

@kkysen I just realized that the Docker images probably don't get rebuilt as a result of this pipeline; I updated the scripts that build Docker images, and then the failure is happening because the CI runs using the old Docker images.

I think this may be easiest to fix by going ahead and switching away from Azure pipelines. I'll work towards that.

@kkysen
Copy link
Contributor

kkysen commented May 8, 2025

Side note: analysis/tests/minimal/reference_pdg.bc was checked in despite being .gitignored. It doesn't look like that test is using "compare" mode, so I don't think that file is being used, only written to. Nonetheless, I moved it across to unstable/analysis/tests/minimal/reference_pdg.bc. If there's any potential cleanup there, it can happen separately from this PR.

It's used in a test here:

let pdg_path: PathBuf = "../analysis/tests/minimal/reference_pdg.bc".into();

As long as you keep the relative paths the same, it should still work fine. We in general gitignored *.bc files but added this one manually for this one test.

@kkysen
Copy link
Contributor

kkysen commented May 8, 2025

@kkysen I just realized that the Docker images probably don't get rebuilt as a result of this pipeline; I updated the scripts that build Docker images, and then the failure is happening because the CI runs using the old Docker images.

I think this may be easiest to fix by going ahead and switching away from Azure pipelines. I'll work towards that.

Oh yes, that's always been one of the very annoying pain points with Azure pipelines. We have to update the docker images separately, and so we can't test multiple in CI at the same time either. One of the many reasons I've always wanted to move away from Azure pipelines. I agree it's probably easier to just move to GitHub Actions in one go and things will work better there. Thanks again for helping with all of this!

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.

2 participants