-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
riscv-gnu-toolchain-*: init at 2023.01.04 #214185
riscv-gnu-toolchain-*: init at 2023.01.04 #214185
Conversation
maintainers/maintainer-list.nix
Outdated
genericnerdyusername = { | ||
name = "GenericNerdyUsername"; | ||
email = "genericnerdyusername@proton.me"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- cd $(srcdir) && \ | ||
- flock `git rev-parse --git-dir`/config git submodule init $(dir $@) && \ | ||
- flock `git rev-parse --git-dir`/config git submodule update $(dir $@) | ||
+ true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+ true |
Isn't this the same?
|
||
nativeBuildInputs = [ autoconf automake flock curl python3 gawk bison flex texinfo gperf ]; | ||
buildInputs = [ libmpc libtool expat.dev gmp.dev mpfr.dev zlib ]; | ||
hardeningDisable = [ "format" ]; | ||
|
||
patches = [ ./nodotgit.patch ]; | ||
configureFlags = [ | ||
"--with-arch=${isa}" | ||
] ++ | ||
srcFlags ++ | ||
lib.optional withLinux "--enable-linux"; | ||
|
||
preConfigure = "patchShebangs ."; | ||
|
||
src = fetchFromGitHub { | ||
owner = "riscv-collab"; | ||
repo = "riscv-gnu-toolchain"; | ||
rev = version; | ||
hash = "sha256-PXPQ/ho+fOudZRErnyQZTVFdceWGBF+geo+3y0tkNm4="; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nativeBuildInputs = [ autoconf automake flock curl python3 gawk bison flex texinfo gperf ]; | |
buildInputs = [ libmpc libtool expat.dev gmp.dev mpfr.dev zlib ]; | |
hardeningDisable = [ "format" ]; | |
patches = [ ./nodotgit.patch ]; | |
configureFlags = [ | |
"--with-arch=${isa}" | |
] ++ | |
srcFlags ++ | |
lib.optional withLinux "--enable-linux"; | |
preConfigure = "patchShebangs ."; | |
src = fetchFromGitHub { | |
owner = "riscv-collab"; | |
repo = "riscv-gnu-toolchain"; | |
rev = version; | |
hash = "sha256-PXPQ/ho+fOudZRErnyQZTVFdceWGBF+geo+3y0tkNm4="; | |
}; | |
src = fetchFromGitHub { | |
owner = "riscv-collab"; | |
repo = "riscv-gnu-toolchain"; | |
rev = version; | |
hash = "sha256-PXPQ/ho+fOudZRErnyQZTVFdceWGBF+geo+3y0tkNm4="; | |
}; | |
nativeBuildInputs = [ autoconf automake flock curl python3 gawk bison flex texinfo gperf ]; | |
buildInputs = [ libmpc libtool expat.dev gmp.dev mpfr.dev zlib ]; | |
hardeningDisable = [ "format" ]; | |
patches = [ ./nodotgit.patch ]; | |
configureFlags = [ | |
"--with-arch=${isa}" | |
] | |
++ srcFlags | |
++ lib.optional withLinux "--enable-linux"; | |
preConfigure = "patchShebangs ."; |
Can we be more specific with files need patching? Maybe some directory?
Why do we need curl in PATH? We cannot download things anyway.
I think we can drop the .dev suffix in buildInputs.
Also patching should be done in postPatch if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turns out the patching isn't necessary, its just leftover from when i was trying to get it working
one of curl, wget and ftp is needed for the configure
, srcs ? { | ||
binutils = { | ||
url = "mirror://gnu/binutils/binutils-2.39.tar.bz2"; | ||
hash = "sha256-2iSoT+8iAQLdJAQt8G/eqFHCYUpTd/hu/6KPM7exYUg="; | ||
}; | ||
dejagnu = { | ||
url = "mirror://gnu/dejagnu/dejagnu-1.6.3.tar.gz"; | ||
hash = "sha256-h9rvrNeVi0pp+IxoVtvRY0JhljxBQHnQw3H1ic1mouM="; | ||
}; | ||
gcc = gcc12.cc.src; | ||
glibc = glibc.src; | ||
newlib = newlib.src; | ||
gdb = gdb.src; | ||
qemu = qemu.src; | ||
musl = musl.src; | ||
spike = spike.src; | ||
pk = riscv-pk.src; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this out of the inputs. It is not really extensible especially when you want to change one variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you not use pkg.override (old: new: {srcs=old.srcs//{new-pkg=new-pkg-src}})
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit hacky. Normally src and srcs are exclusive and I would rather expect that overwriting gcc and its source would affect it. Generally it is not such a great idea to put big defaults behind ?
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better mechanism, or is the ability to change the sources uneeded?
srcs_fetched = lib.mapAttrs (name: value: if value ? outPath then value else fetchurl value) srcs; | ||
srcs_extracted = lib.mapAttrs (name: value: extract value) srcs_fetched; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For which src are we doing this? Maybe we can just change that one package to avoid this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
three of them use fetchFromGitHub
, and I'm not aware of a way to make that use fetchurl
instead of fetchzip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to use fetchurl for github sources or am I mixing things up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some use fetchFromGitHub
, and some use fetchurl
. This means we have a mix of compressed and uncompressed sources
buildInputs = [ libmpc libtool expat.dev gmp.dev mpfr.dev zlib ]; | ||
hardeningDisable = [ "format" ]; | ||
|
||
patches = [ ./nodotgit.patch ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
patches = [ ./nodotgit.patch ]; | |
patches = [ ./no-dot-git.patch ]; |
9108d62
to
269d8d3
Compare
Everything mentioned in the review should be fixed now |
722b21e
to
015829a
Compare
Ready to merge, i think @SuperSandro2000 |
2b3c9fe
to
c8d0e83
Compare
c8d0e83
to
f0d0dca
Compare
…lchain-rv64gc-linux: init at 2023.01.04
f0d0dca
to
2542e05
Compare
#230160 removes the need for this |
Description of changes
Added a toolchain which targets
riscv(64/32)-unkown-elf
. This is different topkgsCross.riscv(64/32)-embedded
, as that targetsriscv(64/32)-none-elf
.Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notesRelevant: #213222