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 manifest for cargo-nextest #518

Merged
merged 1 commit into from
Jun 9, 2024
Merged

Add manifest for cargo-nextest #518

merged 1 commit into from
Jun 9, 2024

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Jun 8, 2024

Closes #487

This is basically a revert of #183 and a fork of #182 (changed to apply only for nextest, see my comment in it for the reason).
This improves security, performance, robustness of installation.

cc @sunshowers @jayvdb

Comment on lines +245 to +246
# musl build of nextest is slow, so use glibc build if host_env is gnu.
# https://github.com/taiki-e/install-action/issues/13
Copy link
Owner Author

Choose a reason for hiding this comment

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

@sunshowers It has been over a year since the update to musl 1.2 in rustc, is my understanding correct that this performance issue still exists?

@taiki-e taiki-e merged commit 5a6e4c7 into main Jun 9, 2024
29 checks passed
@taiki-e taiki-e deleted the dev-nextest-no-binstall branch June 9, 2024 07:41
@seanmonstar
Copy link

seanmonstar commented Jun 11, 2024

I don't know if it's related, but the timing lines up: as of 2 days ago, reqwest CI started failing on i686-pc-windows-gnu, and 86_64. It seems to fail to install the correct version of nextest: https://github.com/seanmonstar/reqwest/actions/runs/9438070294/job/26022148081#step:9:22

(If unrelated, I can move this to a new issue.)

@taiki-e
Copy link
Owner Author

taiki-e commented Jun 11, 2024

A real problem is that msys64's bash is used here, which seems to introduce a lot of differences from the default one, especially that HOME will be a different one on the string.

https://github.com/seanmonstar/reqwest/actions/runs/9438070294/job/26022148081#step:4:2

  echo "C:\msys64\mingw32\bin" >> $GITHUB_PATH
  echo "C:\msys64\usr\bin" >> $GITHUB_PATH

Default one is: https://github.com/taiki-e/install-action/actions/runs/9465237915/job/26074336422#step:5:18

  shell: C:\Program Files\Git\bin\bash.EXE --noprofile --norc -e -o pipefail {0}

$HOME on bash is /c/Users/runneradmin.

When msys64 is used: https://github.com/seanmonstar/reqwest/actions/runs/9438070294/job/26022148081#step:8:17

  shell: C:\msys64\usr\bin\bash.EXE --noprofile --norc -e -o pipefail {0}

$HOME on bash is /home/runneradmin.

And the error you saw is due to the fact that a path starting /home/runneradmin/* that added to PATH by using the GitHub-provided mechanism for adding PATH (GITHUB_PATH) is not recognized by the msys64 bash. (a path starting /c/Users/runneradmin/* is recognized)

This will be worked around in #533.

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.

Creating a manifest for nextest
3 participants