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

Bump rustc version #2253

Closed
wants to merge 3 commits into from
Closed

Bump rustc version #2253

wants to merge 3 commits into from

Conversation

osa1
Copy link
Contributor

@osa1 osa1 commented Jan 13, 2021

This PR bumps rustc version which enables cargo's build-std feature
(#2251) and also allows us to remove some feature flags that became
stable in the meantime (ptr_offset_from, maybe others too).

(build-std feature exists in the current version too, but it's too
buggy or too different than the currently documented version, so I
couldn't use it)

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

Confirmed that upgrading to LLVM 11 fixes the issue.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

In general we need to make sure rustc and wasm-ld (and other LLVM tools if we use them) will be the same version, otherwise as we've just seen incompatibility can cause compile errors only in some cases. I think in theory it could even cause runtime bugs.

I'm trying to find out if there's a reliable way to find out what version of LLVM a given rustc binary is using.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

I can run nix-build -A tests --max-jobs 12 --pure locally just fine but CI fails to build. @nomeata any ideas?

@nomeata
Copy link
Collaborator

nomeata commented Jan 13, 2021

If you follow the “details” link next to the evaluation job, you see the Hydra view with all the individual jobs. There, the tests have not run yet, and the failing jobs (at the moment) are related to darwin stuff.

So likely, the tests will also pass on CI, once they will be built.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

So likely, the tests will also pass on CI, once they will be built.

Right, my question was whether the build failures look familiar.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

Should I try to bump nixpkgs maybe?

@nomeata
Copy link
Collaborator

nomeata commented Jan 13, 2021

CMake Error at CMakeLists.txt:4 (message):
  libc++abi now requires being built in a monorepo layout with libcxx
  available
  
builder for '/nix/store/25020dfwps2ddzkh9g8464bxhi3n2wvd-libc++abi-11.0.0rc2.drv' failed with exit code 1

Maybe worth a try. Might also be worth seeing whether clang11 builds in plain nixpkgs master on darwin.

@dfinity-ci
Copy link

In terms of gas, 3 tests improved and the mean change is -1.1%.
In terms of size, 3 tests regressed and the mean change is +0.1%.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

In terms of gas, 3 tests improved and the mean change is -1.1%.

Thanks, LLVM

Comment on lines +99 to +100
"rev": "e6a184aca0d0d3252a516c317afbfa30fcc7e5d7",
"sha256": "066nkgr7snnlk7q85xz38mxrfbd36b9pasdrwpbc1n6ifbx1q9mz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You changed the commit to a rev on master, without chanigng the branch field. Git doesn't tell you that, but it’ll be confusing for the next person to look at this. So before merging, make sure that the branch field correctly says which branch the revision is from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

We may not want to change the branch, I mainly wanted to see if using the latest version will fix build issues on Darwin.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, that’s what I gathered. Just leaving this comment so that this doesn't slip through in the end.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

rustc PR that fixes showing LLVM version used in rustc -vV: rust-lang/rust#80981

@nomeata
Copy link
Collaborator

nomeata commented Jan 13, 2021

The rts job builds on linux! So that’s promising, isn’t it?

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

It was building before nixpkgs bump too. It even passed the tests on my system.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

Unfortunately shell doesn't work for me anymore. niv doesn't build with recent GHCs, even the tip of the master branch. It's not clear what the supported version of GHC is, README doesn't mention it.

@nomeata
Copy link
Collaborator

nomeata commented Jan 13, 2021

Yes, that’s the problem with trying to upgrade nixpkgs to solve one issue, one suddenly has plenty of other issues.

What was the problem we were trying to solve originally by upgrading? LLVM 11 building on darwin? If so, what I’d try is to copy master’s LLVM files to release-20.09 and see if that works.

@osa1
Copy link
Contributor Author

osa1 commented Jan 13, 2021

Original problem was some kind of build error on Darwin, but I don't remember if it was LLVM or something else.

what I’d try is to copy master’s LLVM files to release-20.09 and see if that works.

I need to clone nixpkgs, make the changes, and point to my fork in sources.json, right?

@nomeata
Copy link
Collaborator

nomeata commented Jan 13, 2021

I need to clone nixpkgs, make the changes, and point to my fork in sources.json, right?

For local experimentation, you can use NIV_OVERRIDE_nixpkgs=../path/to/your/nixpkgs, but on CI, yes. You can use a branch on https://github.com/dfinity-lab/nixpkgs for that

@nomeata
Copy link
Collaborator

nomeata commented Aug 25, 2021

I guess after #2542 we can try this again? Might be worth closing this rather than fixing the conflicts, though.

@osa1
Copy link
Contributor Author

osa1 commented Nov 8, 2021

We updated rustc in #2761, closing.

@osa1 osa1 closed this Nov 8, 2021
@osa1 osa1 deleted the osa1/bump_rustc branch November 8, 2021 06:51
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.

3 participants