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

Add a dist_url_override config key #1509

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

fasterthanlime
Copy link
Contributor

...for quick iteration on cargo-dist itself.

The rationale is explained in the code comments itself.

I have tested this on Linux, via https://github.com/fasterthanlime/bye/releases/tag/v0.6.0 — I have not tested the powershell version yet, lemme do that before taking this PR out of draft.

@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch 5 times, most recently from d97bb5e to adb1292 Compare November 1, 2024 15:47
@fasterthanlime fasterthanlime changed the base branch from main to strict-undefined-behavior November 1, 2024 15:47
@fasterthanlime fasterthanlime force-pushed the strict-undefined-behavior branch from 0c6a265 to eb4e8ee Compare November 1, 2024 15:48
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch 3 times, most recently from 4bd1af1 to 6dc0fec Compare November 1, 2024 16:04
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch 2 times, most recently from 72f0eed to dbef951 Compare November 1, 2024 16:23
@ashleygwilliams ashleygwilliams added this to the 0.26.0 milestone Nov 1, 2024
@fasterthanlime fasterthanlime force-pushed the strict-undefined-behavior branch from c53a91e to 3449d7f Compare November 4, 2024 17:56
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from 8b8d391 to 86341d0 Compare November 4, 2024 18:01
@fasterthanlime fasterthanlime force-pushed the strict-undefined-behavior branch 6 times, most recently from 6b54bd3 to 4c88c10 Compare November 5, 2024 13:39
Base automatically changed from strict-undefined-behavior to main November 5, 2024 14:17
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from 86341d0 to f4a4726 Compare November 5, 2024 15:05
@fasterthanlime fasterthanlime changed the base branch from main to use-checksum-in-installers November 5, 2024 15:06
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from f4a4726 to b7c87e9 Compare November 5, 2024 15:54
@fasterthanlime fasterthanlime force-pushed the use-checksum-in-installers branch from 06c6ee4 to 05c6466 Compare November 6, 2024 14:41
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch 3 times, most recently from 06afd24 to b09e9a7 Compare November 6, 2024 15:22
@fasterthanlime fasterthanlime marked this pull request as ready for review November 6, 2024 18:01
@fasterthanlime fasterthanlime force-pushed the use-checksum-in-installers branch from 05c6466 to 34ce732 Compare November 6, 2024 18:08
Base automatically changed from use-checksum-in-installers to main November 6, 2024 18:19
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from b09e9a7 to 4a7df3a Compare November 6, 2024 18:20
@fasterthanlime fasterthanlime changed the title Add a cargo_dist_url_override key Add a dist_url_override config key Nov 6, 2024
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch 3 times, most recently from f44f975 to 8f9afc5 Compare November 6, 2024 19:00
Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. This'll be handy for testing, and may end up being useful for other people too.

@@ -425,6 +425,7 @@ dependencies = [
"mach_object",
"miette 7.2.0",
"newline-converter",
"schemars",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to anything else but I'm fascinated by this surprise Cargo.lock change.

cargo-dist/src/backend/ci/mod.rs Show resolved Hide resolved
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from 8f9afc5 to 378f1c2 Compare November 6, 2024 19:42
@fasterthanlime fasterthanlime force-pushed the cargo-dist-url-override branch from 6b00dad to f8e0688 Compare November 6, 2024 20:06
@fasterthanlime
Copy link
Contributor Author

Note that the config key is named cargo_dist_url_override, following cargo_dist_version, we can add aliases for dist_-prefix versions later (serde makes that easy).

Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround on that fix. Just running a test now. https://github.com/mistydemeo/cargodisttest/actions/runs/11711074752

@@ -122,7 +122,8 @@ jobs:
with:
submodules: recursive
- name: Install dist
run: ${{ matrix.install_dist }}
shell: ${{ matrix.install_dist.shell }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably have to regenerate our own release YAML now.

@fasterthanlime
Copy link
Contributor Author

Thanks for the quick turnaround on that fix. Just running a test now. https://github.com/mistydemeo/cargodisttest/actions/runs/11711074752

I'm also running a test using the recross branch (cf. #1529)

@fasterthanlime
Copy link
Contributor Author

Thanks for the quick turnaround on that fix. Just running a test now. https://github.com/mistydemeo/cargodisttest/actions/runs/11711074752

That test was successful and the tests are green. Merging!

@fasterthanlime fasterthanlime merged commit f53dd6d into main Nov 6, 2024
10 checks passed
@fasterthanlime fasterthanlime deleted the cargo-dist-url-override branch November 6, 2024 21:42
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.

3 participants