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

Pin rustc version to nightly-2021-04-27 in CI #581

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Pin rustc version to nightly-2021-04-27 in CI #581

merged 2 commits into from
Jul 27, 2021

Conversation

gadomski
Copy link
Member

@gadomski gadomski commented Jul 27, 2021

Related Issue(s): #577 (comment)

Description: When orjson doesn't have a wheel available for a given OS+Python, we have to build it from source during CI. This requires the rust compiler (and associated tooling). There was a minor regression in the output of the --version command in one rust tool (cargo) which caused the orjson build chain to break. When rust-lang/cargo#9727 is incorporated back into nightly Rust releases we can unpin this toolchain and revert it back to nightly.

The check that was failing because of the regression:
https:/PyO3/maturin/blob/5f134e3a4adbbf21f04c10f1dc9722742156f959/maturin/__init__.py#L114-L122

PR Checklist:

  • Code is formatted (run pre-commit run --all-files)
  • Tests pass (run scripts/test)
  • [ ] Documentation has been updated to reflect changes, if applicable
  • [ ] This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@codecov-commenter
Copy link

Codecov Report

Merging #581 (81705ed) into main (b8259fe) will not change coverage.
The diff coverage is n/a.

❗ Current head 81705ed differs from pull request most recent head 098eeef. Consider uploading reports for the commit 098eeef to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #581   +/-   ##
=======================================
  Coverage   94.69%   94.69%           
=======================================
  Files          75       75           
  Lines       10776    10776           
  Branches     1053     1053           
=======================================
  Hits        10204    10204           
  Misses        396      396           
  Partials      176      176           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8259fe...098eeef. Read the comment docs.

When orjson doesn't have a wheel available for a given OS+Python, we
have to build it from source during CI. This requires the rust compiler
(and associated tooling). There was a minor regression in the output of
the `--version` command in one rust tool (`cargo`) which caused the
`orjson` build chain to break. When
rust-lang/cargo#9727 is incorporated back into
nightly Rust releases we can unpin this toolchain and revert it back to
`nightly`.

The check that was failing because of the regression:
https:/PyO3/maturin/blob/5f134e3a4adbbf21f04c10f1dc9722742156f959/maturin/__init__.py#L114-L122
Copy link
Contributor

@duckontheweb duckontheweb 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 tracking this down @gadomski!

Given that we're only using this until the orjson wheels are more available and the fact that the nightly toolchain is inherently unstable I'm thinking we should just keep this pinned to a specific nightly version. What do you think?

@duckontheweb duckontheweb merged commit b9c47e6 into stac-utils:main Jul 27, 2021
@gadomski gadomski deleted the fix/ci-without-orjson-wheels branch July 27, 2021 18:37
@gadomski
Copy link
Member Author

I think we should keep it pinned until something breaks again, then update the pin to another nightly that works. Tracking nightly w/o a pin feels like a recipe for intermittent errors.

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.

4 participants