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

Remove resolver=2 from Cargo.toml and add it to the Windows build #26706

Merged
merged 3 commits into from Jul 22, 2022
Merged

Remove resolver=2 from Cargo.toml and add it to the Windows build #26706

merged 3 commits into from Jul 22, 2022

Conversation

willhickey
Copy link
Contributor

@willhickey willhickey commented Jul 20, 2022

See discussion on PR #26555

Problem

PR #24196 added resolver = "2" to Cargo.toml. That fixed the broken Windows build but is causing other problems (see #22603)

Summary of Changes

  • Remove resolver = "2" from Cargo.toml
  • Add steps to the Windows build that add resolver = "2" to the local copy of Cargo.toml immediately before the build.

context #22603

@bw-solana bw-solana self-requested a review July 20, 2022 21:47
bw-solana
bw-solana previously approved these changes Jul 20, 2022
Copy link
Contributor

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 64 to 68
echo ${{ secrets.GITHUB_TOKEN }} | gh auth login --with-token
git config --global user.email "<>"
git config --global user.name "Github Action Bot"
git add -u
git commit -m "Add resolver=2"
Copy link
Member

@yihau yihau Jul 21, 2022

Choose a reason for hiding this comment

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

maybe we don't want to commit here. ci/publish-tarball.sh creates a version.yml depends on the latest commit. if we do a new commit, I think it makes windows's version.yml will always different from linux and mac.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good catch. I moved the logic into ci/publish-tarball.sh so it's after the git reset --hard

@mergify mergify bot dismissed bw-solana’s stale review July 21, 2022 19:20

Pull request has been modified.

Copy link
Member

@yihau yihau left a comment

Choose a reason for hiding this comment

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

👍

@willhickey willhickey merged commit 2f6f5b1 into solana-labs:master Jul 22, 2022
@ryoqun
Copy link
Member

ryoqun commented Aug 4, 2022

@willhickey @yihau how about backporting this to v1.10 branch? do you see any concerns? this pr has been in for awhile in master and causing no problem? as a context, I'm intending to ship #26555 to v1.10.

@yihau
Copy link
Member

yihau commented Aug 4, 2022

just installed an edge bin on windows. I think it works fine.

mergify bot pushed a commit that referenced this pull request Aug 4, 2022
mergify bot added a commit that referenced this pull request Aug 5, 2022
…ckport #26706) (#26928)

Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)

(cherry picked from commit 2f6f5b1)

Co-authored-by: Will Hickey <will.hickey@solana.com>
@ryoqun
Copy link
Member

ryoqun commented Aug 9, 2022

@willhickey @yihau oops. it looks like this pr isn't needed for #26555 actually. sorry. I'm reverting this.

I was seeing odd error because, resolver = "2" is part of the newly added [patch] section. this is a foot gun...

i.e. this works:

 # This prevents a Travis CI error when building for Windows.
 resolver = "2"
+
+# for details, see https://github.com/solana-labs/crossbeam/commit/fd279d707025f0e60951e429bf778b4813d1b6bf
+[patch.crates-io]
+crossbeam-epoch = { git = "https://github.com/solana-labs/crossbeam", rev = "fd279d707025f0e60951e429bf778b4813d1b6bf" 

this won't:

+
+# for details, see https://github.com/solana-labs/crossbeam/commit/fd279d707025f0e60951e429bf778b4813d1b6bf
+[patch.crates-io]
+crossbeam-epoch = { git = "https://github.com/solana-labs/crossbeam", rev = "fd279d707025f0e60951e429bf778b4813d1b6bf" 

 # This prevents a Travis CI error when building for Windows. <= Even this comment doesn't terminate the preceeding patch section parsing context...
 resolver = "2"  # <= this is now meaning to patch a non-existing crate called `resolver`... I'm dumb
$ cargo clippy
    Updating crates.io index
error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
  patch for `resolver` in `https://github.com/rust-lang/crates.io-index` failed to resolve

Caused by:
  The patch location `registry `crates-io`` contains a `resolver` package with versions `0.1.0, 0.1.1`, but the patch definition requires `^2`.
  Check that the version in the patch location is what you expect, and update the patch definition to match.

ryoqun added a commit that referenced this pull request Aug 9, 2022
ryoqun added a commit that referenced this pull request Aug 9, 2022
…uild" (#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)"

This reverts commit 2f6f5b1.
mergify bot pushed a commit that referenced this pull request Aug 9, 2022
…uild" (#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)"

This reverts commit 2f6f5b1.

(cherry picked from commit ecda3be)
mergify bot added a commit that referenced this pull request Aug 9, 2022
…uild" (backport #27011) (#27019)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build" (#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)"

This reverts commit 2f6f5b1.

(cherry picked from commit ecda3be)

Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
mergify bot pushed a commit that referenced this pull request Aug 9, 2022
…uild" (#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)"

This reverts commit 2f6f5b1.

(cherry picked from commit ecda3be)
mergify bot added a commit that referenced this pull request Aug 9, 2022
…uild" (backport #27011) (#27024)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build" (#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (#26706)"

This reverts commit 2f6f5b1.

(cherry picked from commit ecda3be)

Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
xiangzhu70 pushed a commit to xiangzhu70/solana that referenced this pull request Aug 17, 2022
…uild" (solana-labs#27011)

Revert "Remove resolver=2 from Cargo.toml and add it to the Windows build (solana-labs#26706)"

This reverts commit 2f6f5b1.
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.

None yet

4 participants