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
65 changes: 63 additions & 2 deletions .github/workflows/ruby-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ jobs:
run: |
brew install gnu-tar
echo "/usr/local/opt/gnu-tar/libexec/gnubin" >> $GITHUB_PATH
- name: Install cargo-cross
if: runner.os == 'Linux'
run: cargo install cross --version 0.2.1
- uses: ./.github/actions/os-version
id: os_version
- name: Cache entire extractor
Expand Down Expand Up @@ -78,8 +81,13 @@ jobs:
- name: Run tests
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: cd extractor && cargo test --verbose
- name: Release build
if: steps.cache-extractor.outputs.cache-hit != 'true'
# On linux, build the extractor via cross in a centos7 container.
# This ensures we don't depend on glibc > 2.17.
- name: Release build (linux)
if: steps.cache-extractor.outputs.cache-hit != 'true' && runner.os == 'Linux'
run: cd extractor && cross build --release
- name: Release build (windows and macos)
if: steps.cache-extractor.outputs.cache-hit != 'true' && runner.os != 'Linux'
run: cd extractor && cargo build --release
- name: Generate dbscheme
if: ${{ matrix.os == 'ubuntu-latest' && steps.cache-extractor.outputs.cache-hit != 'true'}}
Expand Down Expand Up @@ -227,3 +235,56 @@ jobs:
shell: bash
run: |
codeql database analyze --search-path "${{ runner.temp }}/ruby-bundle" --format=sarifv2.1.0 --output=out.sarif ../database ruby-code-scanning.qls

# This is a copy of the 'test' job that runs in a centos7 container.
# This tests that the extractor works correctly on systems with an old glibc.
test-centos7:
defaults:
run:
working-directory: ${{ github.workspace }}
strategy:
fail-fast: false
runs-on: ubuntu-latest
container:
image: centos:centos7
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
needs: [package]
steps:
- name: Install gh cli
run: |
yum-config-manager --add-repo https://cli.github.com/packages/rpm/gh-cli.repo
# fetch-codeql requires unzip and jq
# jq is available in epel-release (https://docs.fedoraproject.org/en-US/epel/)
yum install -y gh unzip epel-release
yum install -y jq
- uses: actions/checkout@v3
- 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.

# https://github.com/actions/runner/issues/2185
- name: Ensure temp directory exists
run: mkdir -p /tmp

- name: Download Ruby bundle
uses: actions/download-artifact@v3
with:
name: codeql-ruby-bundle
path: /tmp
- name: Unzip Ruby bundle
shell: bash
run: unzip -q -d /tmp/ruby-bundle /tmp/codeql-ruby-bundle.zip
aibaars marked this conversation as resolved.
Show resolved Hide resolved

- name: Run QL test
shell: bash
run: |
codeql test run --search-path /tmp/ruby-bundle --additional-packs /tmp/ruby-bundle ruby/ql/test/library-tests/ast/constants/
- name: Create database
shell: bash
run: |
codeql database create --search-path /tmp/ruby-bundle --language ruby --source-root ruby/ql/test/library-tests/ast/constants/ ../database
- name: Analyze database
shell: bash
run: |
codeql database analyze --search-path /tmp/ruby-bundle --format=sarifv2.1.0 --output=out.sarif ../database ruby-code-scanning.qls
4 changes: 2 additions & 2 deletions ruby/doc/HOWTO.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This document contains information about common development tasks.
[Install Rust](https://www.rust-lang.org/tools/install), then run:

```bash
cargo build --release
(cd extractor && cargo build --release)
```

## Generating the database schema and QL library
Expand All @@ -16,7 +16,7 @@ The generated `ql/lib/ruby.dbscheme` and `ql/lib/codeql/ruby/ast/internal/TreeSi

```bash
# Run the generator
cargo run --release -p ruby-generator -- --dbscheme ql/lib/ruby.dbscheme --library ql/lib/codeql/ruby/ast/internal/TreeSitter.qll
(cd extractor && cargo run --release --bin generator -- --dbscheme ../ql/lib/ruby.dbscheme --library ../ql/lib/codeql/ruby/ast/internal/TreeSitter.qll)
# Then auto-format the QL library
codeql query format -i ql/lib/codeql/ruby/ast/internal/TreeSitter.qll
```
Expand Down
2 changes: 1 addition & 1 deletion ruby/extractor/rust-toolchain.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
# extractor. It is set to the lowest version of Rust we want to support.

[toolchain]
channel = "1.54"
channel = "1.68"
profile = "minimal"
components = [ "rustfmt" ]
2 changes: 2 additions & 0 deletions ruby/extractor/src/bin/extractor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ fn scan_erb(
(result, line_breaks)
}

/// Advance `index` to the next non-whitespace character.
/// Newlines are **not** considered whitespace.
fn skip_space(content: &[u8], index: usize) -> usize {
let mut index = index;
while index < content.len() {
Expand Down