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

Ruby: Bump rust toolchain to 1.68 #12529

Merged
merged 11 commits into from
Mar 22, 2023

Conversation

hmac
Copy link
Contributor

@hmac hmac commented Mar 15, 2023

This should not affect our support for systems with older glibc versions (e.g. Centos 7), and it will allow us to use newer library versions and Cargo features.

In addition, I've added a specific test that runs the extractor in a Centos 7 container, to ensure that we don't break this compatibility in future.

@github-actions github-actions bot added the Ruby label Mar 15, 2023
@hmac hmac force-pushed the ruby-extractor-bump-rust-version branch from 6bcdd4f to 9eda668 Compare March 15, 2023 08:58
@hmac hmac force-pushed the ruby-extractor-bump-rust-version branch from 41e3abb to e7ead76 Compare March 15, 2023 23:04
@hmac hmac marked this pull request as ready for review March 15, 2023 23:43
@hmac hmac requested a review from a team as a code owner March 15, 2023 23:43
@hmac hmac added the no-change-note-required This PR does not need a change note label Mar 15, 2023
@hmac hmac force-pushed the ruby-extractor-bump-rust-version branch from 48a6053 to d4020ad Compare March 16, 2023 21:38
@calumgrant calumgrant requested a review from aibaars March 20, 2023 09:44
aibaars
aibaars previously approved these changes Mar 21, 2023
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

Looks good to me!

- name: Fetch CodeQL
uses: ./.github/actions/fetch-codeql

# Due to a bug in Actions, we can't use runner.temp here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that ${{ runner.temp }} does not work? It looks like things are supposed to be fixed in actions/runner#1762 .

Copy link
Contributor

@aibaars aibaars Mar 21, 2023

Choose a reason for hiding this comment

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

Looking at the failed run https://github.com/github/codeql/actions/runs/4442367661/jobs/7798560621 it seems the bug was only half-fixed by actions/runner#1762 ...

The use of ${{ github.temp }} in the with clause of the download step was expanded correctly: `https://github.com/github/codeql/actions/runs/4442367661/jobs/7798560621

  with:
    name: codeql-ruby-bundle
    path: /home/runner/work/_temp

but the variable was expanded wrongly in the run: step:

unzip -q -d "/home/runner/work/_temp/ruby-bundle" "/home/runner/work/_temp/codeql-ruby-bundle.zip"

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that others have tried to work around the problem by using the environment variables instead. Might be worth a shot to use "$RUNNER_TEMP" in the run: steps. This is actually better in general and avoids potential path injection attacks if a variable contains a " (unlikely for the runner.temp variable, but plausible for some user provided ones).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll give that a try and see if it works.

@aibaars aibaars merged commit 65d129d into github:main Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants