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

Fix cycle detected in Darwin->Linux cross GCC #96404

Merged
merged 95 commits into from Aug 26, 2020

Conversation

matthewbauer
Copy link
Member

Fixes #88213

ThibautMarty and others added 30 commits April 28, 2020 19:13
They can be caught with `nixos-option -r` on an empty ({...}:{}) NixOS
configuration.
This reverts d9feea5 with some slight modifications to work with
other changes since then.

Fixes NixOS#88213.
This doesn’t work with latest nccl anymore.
This appears to avoid requiring KVM when it’s not available. This is
what I originally though -cpu host did. Unfortunately not much
documentation available from the QEMU side on this, but this appears
to square with help:

$ qemu-system-x86 -cpu help
...
x86 host                  KVM processor with all supported host features
x86 max                   Enables all features supported by the accelerator in the current host
...

Whether we actually want to support this not clear, since this only
happens when your CPU doesn’t have full KVM support. Some Nix builders
are lying about kvm support though. Things aren’t too slow without it
though.

Fixes NixOS#85394

Alternative to NixOS#83920
This way we not accidentally use introduce/use global variables.
Also it explictly mark the code for the mypy type checker.
- Less code
- more thread-safe according to @flokli
Appearantly this is used in tests
This is more robust than depending on the channel, though the version
should only matter if the configuration phase fails.
This also switches to the intended version for `chromium` which should
be higher since M85 is in the stable channel.

Thanks `@volth` for pointing this out.
@matthewbauer matthewbauer changed the base branch from master to staging August 26, 2020 21:16
@matthewbauer matthewbauer merged commit 25ac498 into NixOS:staging Aug 26, 2020
@Gaelan
Copy link
Contributor

Gaelan commented Aug 26, 2020

@matthewbauer thanks for picking this up

@jonringer
Copy link
Contributor

umm, was the right branch pushed @matthewbauer ?

@jonringer
Copy link
Contributor

The commit you pushed contains a lot more than just your changes 25ac498

@Mic92
Copy link
Member

Mic92 commented Aug 26, 2020

I think he merged master into staging.

@Mic92
Copy link
Member

Mic92 commented Aug 26, 2020

Which should be fine.

@jonringer
Copy link
Contributor

it will still make git history awkward, and might cause some issues with git bisect

@jonringer
Copy link
Contributor

plus, you generally want to push the changes from master to staging-next, then staging-next into staging, if you're trying to update the staging branches.

@Mic92
Copy link
Member

Mic92 commented Aug 26, 2020

I thought stuff goes to staging first before it goes to staging-next.

@jonringer
Copy link
Contributor

jonringer commented Aug 26, 2020

if you want changes from master to be pushed to staging, generally you push them first to staging-next, then push staging-next into staging

EDIT: if you want the changes from staging to be in master, then you push it to staging-next, do some stabilization, and let hydra build up the cache, then merge into master

@jonringer
Copy link
Contributor

I can't find the original post, @FRidh would know better

@matthewbauer
Copy link
Member Author

matthewbauer commented Aug 26, 2020

Oh ok, so origin/master should merge into origin/staging-next first then origin/staging? Usually I think it went the other way where origin/master merges into origin/staging which eventually merges back into origin/staging-next.

@jonringer
Copy link
Contributor

it does both

staging -> staging-next -> master: to bring changes from staging to master, but allowing for stabilization

master -> staging-next -> staging: to prevent staging and staging-next from diverging from master significantly (this is usually done after staging-next PR is completed, because it should have changes in master already in it)

master -> staging-next: done frequently in staging-next PR/stabilization period So that changes in master can be applied and rebuilt off of changes from staging.

@jonringer
Copy link
Contributor

Like I said before, this probably won't affect "derivations" too much, but it could cause some awkwardness with git bisect creating "false positives" due to messed up git history. Also, it might cause a large amount of merge conflicts for people working on staging-next

@jonringer
Copy link
Contributor

at the very least, the PR should have been open longer than 100s before getting merged

@@ -241,7 +241,7 @@ postInstall() {
# More dependencies with the previous gcc or some libs (gccbug stores the build command line)
rm -rf $out/bin/gccbug

if type "patchelf"; then
if [[ buildConfig == *"linux"* ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

[[ buildConfig == *"linux"* ]] is always false since linux is not a substring of the literal string buildConfig. Did you mean for this to be a variable or something?

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.

cycle detected in Darwin->Linux cross GCC