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

gcc8: init at 8.1.0 #39866

Merged
merged 9 commits into from May 4, 2018
Merged

gcc8: init at 8.1.0 #39866

merged 9 commits into from May 4, 2018

Conversation

Synthetica9
Copy link
Member

@Synthetica9 Synthetica9 commented May 2, 2018

Motivation for this change

GCC 8.1 was just released.

Things done
  • This is still compiling on my machine. I see no reason why it wouldn't finish, but I wanted to get the WIP PR out. Okay; it compiled. Ship it?

  • This does not set GCC8 as the default version.

  • For good measure, this derivation is mostly copied from GCC7

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)

  • Built on platform(s)

    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"

  • Tested execution of all binary files (usually in ./result/bin/)

  • Fits CONTRIBUTING.md.


@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

I vote getting this in the repository first, with separate PR for changing the default (and dealing with any breakage/fixes needed).

@Synthetica9
Copy link
Member Author

Sounds fair enough


src = fetchurl {
url = "mirror://gcc/releases/gcc-${version}/gcc-${version}.tar.xz";
/* sha256 = "0p71bij6bfhzyrs8676a8jmpjsfz392s2rg862sdnsk30jpacb43"; */
Copy link
Member

Choose a reason for hiding this comment

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

Remove the gcc7 hash, please :).

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, ✔️

@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

  • cc @shlevy re:riscV patches and testing (but no need to block on a response or this being resolved)
  • Let's see what borg says first :)
  • I'll try w/musl, but this isn't a blocker either :)

Log:
```
patching sources
applying patch /nix/store/6m27y27zvzsjn1ir4y8mm9nc9xnh2sgx-riscv-no-relax.patch
patching file gcc/config/riscv/riscv.c
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file gcc/config/riscv/riscv.c.rej
patching file gcc/config/riscv/riscv.opt
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file gcc/config/riscv/riscv.opt.rej
patching file gcc/doc/invoke.texi
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
```
@Synthetica9
Copy link
Member Author

@GrahamcOfBorg build gcc8

@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

Consider adding these commits:

(for testing purposes I made a branch that contains this as well as setting gcc8 as default:
https://github.com/dtzWill/nixpkgs/commits/experimental/gcc8)

@Synthetica9
Copy link
Member Author

@dtzWill The drop-self-test patch was available in 7.3.0 (https://git.busybox.net/buildroot/plain/package/gcc/7.3.0), so I don't we should remove that yet (unless it was somehow confirmed by the GCC dev team that it won't be needed)

@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

@dtzWill The drop-self-test patch was available in 7.3.0 (https://git.busybox.net/buildroot/plain/package/gcc/7.3.0), so I don't we should remove that yet (unless it was somehow confirmed by the GCC dev team that it won't be needed)

Well since the patch wasn't issued by the GCC dev team I'm not sure why they're the authority to ask :).

But leaving it commented-out works for me-- the real answer is to get some builders testing cross to see if it is needed or not. That can be handled separately, sorry for piling on :).

@Synthetica9
Copy link
Member Author

@dtzWill Nah, it's fine. I'll change the hash to a placeholder value and make the url dependent on the version, but leave it commented.

How's the gcc7->gcc8 switch looking with regards to broken packages?

@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

How's the gcc7->gcc8 switch looking with regards to broken packages?

Not sure--and I won't be able to provide a good answer for this, someone would need to setup a new jobset on Hydra. But that can be done in the follow-up PR that changes the default?

I can confirm that with gcc8 as default we're at least able to build

  • x86_64-linux: through cc-test-wrapper
  • x86_64-musl: through test-wrapper
  • x86_64 -> aarch64-musl:stdenv.cc (no self-tests patch!)
  • x86_64 -> aarch64-glibc: errror building glibc itself D:

(I have tested very little beyond basics so far, so dunno re:package compatibility at all :))

@dtzWill
Copy link
Member

dtzWill commented May 2, 2018

LGTM, borg is probably having same problem I was having with mirrors (ended up manually using nix-prefetch-url). We can at least make it available for testing with this!

@Synthetica9 Synthetica9 changed the title [WIP] gcc8: init at 8.1.0 gcc8: init at 8.1.0 May 2, 2018
, buildPackages
}:

assert langJava -> zip != null && unzip != null
Copy link
Member

Choose a reason for hiding this comment

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

I thought upstream had removed java support in gcc 6?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah, right you are. Should the arguments stay in place and go unused (apart from maybe an assert not langJava), or should I remove them altogether (

, cloog # unused; just for compat with gcc4, as we override the parameter on some places
seems to suggest that the first option would be more conventional.)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove them completely. AFAICT the only place where we override enableJava et al. is in the gcj6 attribute (which uses gcc6).

@Synthetica9 Synthetica9 changed the title gcc8: init at 8.1.0 [WIP] gcc8: init at 8.1.0 May 3, 2018
@Synthetica9
Copy link
Member Author

@gebner I removed all mentions of Java. I'll also create a PR to do the same for GCC7.

@shlevy
Copy link
Member

shlevy commented May 3, 2018

Unable to test at the moment, but IIRC gcc 8 contains the RISC-V stuff already

@Synthetica9
Copy link
Member Author

@shlevy I ctrl-f'd the release notes for RISC, but it isn't mentioned AFAICS.

@shlevy
Copy link
Member

shlevy commented May 3, 2018

Just checked, both patches are on the 8.1.0 tag on subversion.

, libelf # optional, for link-time optimizations (LTO)
, isl ? null # optional, for the Graphite optimization framework.
, zlib ? null
, pkgconfig ? null
Copy link
Member

Choose a reason for hiding this comment

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

With java support removed, all of these options (pkgconfig until x11Support) are now unused as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

grabs hatchet

Synthetica9 added a commit to Synthetica9/nixpkgs that referenced this pull request May 3, 2018
@Synthetica9 Synthetica9 changed the title [WIP] gcc8: init at 8.1.0 gcc8: init at 8.1.0 May 3, 2018
@dtzWill
Copy link
Member

dtzWill commented May 4, 2018

This is ready to be merged, just waiting to confirm doesn't trigger any mass-rebuilds.

@dtzWill
Copy link
Member

dtzWill commented May 4, 2018

LGTM, apologies if I missed something. Thanks @Synthetica9!

@dtzWill dtzWill merged commit e02dfb5 into NixOS:master May 4, 2018
@Synthetica9
Copy link
Member Author

@dtzWill: I think the next step would be a PR to use GCC8 as the default GCC version? Would it be sufficient to just git cherry-pick dtzWill@2e7772a from master and throw that up for PR?

@dtzWill
Copy link
Member

dtzWill commented May 4, 2018

@Synthetica9 I think so! There definitely is some breakage but we can address that in that PR :).

(and if my commit isn't sufficient for whatever reason that can be discussed/reviewed there as well)

@matthewbauer
Copy link
Member

Hi,

gcc8 is failing on x86_64-darwin:

Job: https://hydra.nixos.org/job/nixpkgs/trunk/gcc8.x86_64-darwin

Not sure what's going on. Clang gives this error:

clang-5.0: error: unsupported option '--gstabs'

@Synthetica9
Copy link
Member Author

@matthewbauer I think it would be best to open a separate issue for that; I don't have access to a macos box.

@dtzWill dtzWill mentioned this pull request Jul 27, 2018
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants