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

gnirehtet: update cargo.lock to fix build with rust 1.64.0 #117139

Closed
wants to merge 2 commits into from

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Dec 2, 2022

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Needs new bottles since Linux is hitting relocation issue:

  ==> Pouring gnirehtet--2.5_1.x86_64_linux.bottle.tar.gz
  Error: cannot normalize PT_NOTE segment: non-contiguous SHT_NOTE sections

However, rebottle attempts fails with Rust 1.64.0+ - https://github.com/Homebrew/homebrew-core/actions/runs/3589552605/jobs/6042109311

Comment on lines 33 to 37
# Building the binary with rust 1.64.0 or later results in an error when running `gnirehtet relay`.
# ERROR Main: Execution error: IO error: Address family not supported by protocol family (os error 47)
# Issue ref: https://github.com/Genymobile/gnirehtet/issues/475
system "rustup-init", "-qy", "--no-modify-path", "--default-toolchain", "1.63.0"
ENV.prepend_path "PATH", HOMEBREW_CACHE/"cargo_cache/bin"
Copy link
Member Author

Choose a reason for hiding this comment

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

If anyone knows of an upstreamable fix for Rust 1.64.0+, then we can patch it in instead.

For now, went with workaround of downgrading Rust version.

@cho-m cho-m added the maintainer feedback Additional maintainers' opinions may be needed label Dec 5, 2022
carlocab
carlocab previously approved these changes Dec 7, 2022
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

What version are our bottles built with? Is the error from building with the wrong version of Rust caught by the test?

@carlocab carlocab added the automerge-skip `brew pr-automerge` will skip this pull request label Dec 7, 2022
@cho-m
Copy link
Member Author

cho-m commented Dec 7, 2022

What version are our bottles built with?

A mix of versions, e.g.

  • Monterey bottles were built in 2b185d9 with Rust 1.56.0
  • Linux bottle was built in 9b2420c with Rust 1.53.0
  • Catalina and older bottles were built in e2b2998 with Rust 1.45.2

Is the error from building with the wrong version of Rust caught by the test?

Yes, test failed in Rust 1.64.0 PR #111434 - https://github.com/Homebrew/homebrew-core/actions/runs/3109704394/jobs/5041504928#step:10:4266

@SMillerDev
Copy link
Member

Last commit in 2020, lots of open trivial PRs I'd just mark it as an unmaintained project

chenrui333
chenrui333 previously approved these changes Dec 26, 2022
@chenrui333
Copy link
Member

Last commit in 2020, lots of open trivial PRs I'd just mark it as an unmaintained project

Raised an issue in the upstream about the project status, we can revisit maybe couple of months later.

@cho-m
Copy link
Member Author

cho-m commented Dec 26, 2022

Upstream has created a PR to update Rust dependencies in Cargo.lock to fix issue. We can try the PR commit instead.

@chenrui333
Copy link
Member

Agree, let's try the upstream PR patch instead.

@chenrui333 chenrui333 dismissed stale reviews from carlocab and themself via cf1a9f8 December 26, 2022 20:18
@BrewTestBot BrewTestBot added rust Rust use is a significant feature of the PR or issue and removed automerge-skip `brew pr-automerge` will skip this pull request labels Dec 26, 2022
@chenrui333 chenrui333 changed the title gnirehtet: use rustup-init to get Rust 1.63.0 to fix build gnirehtet: update cargo.lock to fix build with rust 1.64.0 Dec 26, 2022
@chenrui333 chenrui333 removed the maintainer feedback Additional maintainers' opinions may be needed label Dec 26, 2022
@chenrui333 chenrui333 added the ready to merge PR can be merged once CI is green label Dec 26, 2022
@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@cho-m cho-m deleted the gnirehtet-workaround-build branch October 30, 2023 22:37
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age ready to merge PR can be merged once CI is green rust Rust use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants