[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

ghdl-gcc: init at 4.1.0 #322259

Merged
merged 2 commits into from
Jul 10, 2024
Merged

ghdl-gcc: init at 4.1.0 #322259

merged 2 commits into from
Jul 10, 2024

Conversation

martijnbastiaan
Copy link
Contributor
@martijnbastiaan martijnbastiaan commented Jun 24, 2024 •
edited
Loading

Not truly a new package, but rather a new backend next to the existing mcode and llvm ones. Adding the GCC backend too, as it is the only one that supports code coverage report generation. For more info see: https://devsaurus.github.io/ghdl_gcov/ghdl_gcov.html.

Description of changes

Add ghdl-gcc, GHDL with GCC as a backend.

Things done

  • Run internal tests:
    • nix-build -A ghdl-mcode.passthru.tests
    • nix-build -A ghdl-llvm.passthru.tests
    • nix-build -A ghdl-gcc.passthru.tests
  • Ad-hoc tests: tested in a local nix-shell --pure in combination with lcov.
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Notes

Tags

Maintainers: @Lucus16 @thoughtpolice

Colleagues: @stdefeber @rowanG077

Comments

I'd love to backport this to 24.05.

Add a 👍 reaction to pull requests you find important.

@martijnbastiaan
Copy link
Contributor Author

Does the CI pkgs/by-name warning/error apply to compilers? Or should I move the files there?

@martijnbastiaan
Copy link
Contributor Author

nixpkgs-review pr 322259:

$ git -c fetch.prune=false fetch --no-tags --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/322259/head:refs/nixpkgs-review/1
remote: Enumerating objects: 7460, done.
remote: Counting objects: 100% (4761/4761), done.
remote: Compressing objects: 100% (599/599), done.
remote: Total 7460 (delta 4356), reused 4416 (delta 4119), pack-reused 2699
Receiving objects: 100% (7460/7460), 5.83 MiB | 24.28 MiB/s, done.
Resolving deltas: 100% (5132/5132), completed with 1130 local objects.
From https://github.com/NixOS/nixpkgs
 * [new branch]                master                -> refs/nixpkgs-review/0
 * [new ref]                   refs/pull/322259/head -> refs/nixpkgs-review/1
$ git worktree add /home/martijn/.cache/nixpkgs-review/pr-322259/nixpkgs 525a63b1cd6d0ecc454ac49574b0a7bf4633eefb
Preparing worktree (detached HEAD 525a63b1cd6d)
Updating files: 100% (41803/41803), done.
HEAD is now at 525a63b1cd6d Merge pull request #322175 from r-ryantm/auto-update/kitex
$ nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f /home/martijn/.cache/nixpkgs-review/pr-322259/nixpkgs -qaP --xml --out-path --show-trace --no-allow-import-from-derivation
$ git merge --no-commit --no-ff dca53242136200003e6bb66c1ff2868843764ad3
Auto-merging pkgs/top-level/all-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f /home/martijn/.cache/nixpkgs-review/pr-322259/nixpkgs -qaP --xml --out-path --show-trace --no-allow-import-from-derivation --meta
1 package added:
ghdl-gcc (init at 4.1.0)

7 packages updated:
ghdl-llvm ghdl-mcode python3.11-cocotb python3.11-myhdl-unstable python3.12-cocotb python3.12-myhdl-unstable yosys-ghdl

$ nix build --extra-experimental-features nix-command no-url-literals --no-link --keep-going --no-allow-import-from-derivation --option build-use-sandbox relaxed -f /home/martijn/.cache/nixpkgs-review/pr-322259/build.nix

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/322259

12 packages built:
ghdl ghdl-gcc ghdl-llvm python311Packages.cocotb python311Packages.cocotb.dist python311Packages.myhdl python311Packages.myhdl.dist python312Packages.cocotb python312Packages.cocotb.dist python312Packages.myhdl python312Packages.myhdl.dist yosys-ghdl

$ /nix/store/bypzxxc6bzd4ymr3y36j9fwpfl3a16kd-nix-2.11.1/bin/nix-shell /home/martijn/.cache/nixpkgs-review/pr-322259/shell.nix

@ofborg ofborg bot requested review from thoughtpolice and Lucus16 June 24, 2024 21:22
@Aleksanaa
Copy link
Member

Does the CI pkgs/by-name warning/error apply to compilers? Or should I move the files there?

Yes; Yes

@martijnbastiaan martijnbastiaan force-pushed the add-ghdl-gcc branch 2 times, most recently from eef9543 to aa36e7e Compare July 5, 2024 18:26
@martijnbastiaan
Copy link
Contributor Author

@Aleksanaa Fixed. CI seems good now. Is there anything I can do to help this getting merged?

@martijnbastiaan
Copy link
Contributor Author

Gentle ping @Lucus16 @thoughtpolice

Lucus16
Lucus16 suggested changes Jul 8, 2024
View reviewed changes
Copy link
Contributor
@Lucus16 Lucus16 left a comment

Choose a reason for hiding this comment

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

This mostly looks good, but I think it's important that users do not need to set LIBRARY_PATH manually.

pkgs/by-name/gh/ghdl/test-simple.nix Outdated Show resolved Hide resolved
pkgs/by-name/gh/ghdl/test-simple.nix Outdated Show resolved Hide resolved
pkgs/by-name/gh/ghdl/package.nix Outdated Show resolved Hide resolved
@martijnbastiaan martijnbastiaan force-pushed the add-ghdl-gcc branch 2 times, most recently from bc27c80 to e94e1db Compare July 8, 2024 18:52
@martijnbastiaan @rowanG077
ghdl-gcc: init at 4.1.0
8dbbabe 
Not truly a new package, but rather a new backend next to the existing
`mcode` and `llvm` ones. Adding the GCC backend too, as it is the only
one that supports code coverage report generation. For more info see:
https://devsaurus.github.io/ghdl_gcov/ghdl_gcov.html.

Co-authored-by: Rowan Goemans <goemansrowan@gmail.com>
@martijnbastiaan
Copy link
Contributor Author

@Lucus16 All done

Lucus16
Lucus16 approved these changes Jul 10, 2024
View reviewed changes
Copy link
Contributor
@Lucus16 Lucus16 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@yorickvP yorickvP merged commit 45aa2e2 into NixOS:master Jul 10, 2024
26 checks passed
@martijnbastiaan martijnbastiaan deleted the add-ghdl-gcc branch July 11, 2024 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lucus16 Lucus16 Lucus16 approved these changes

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@martijnbastiaan @Aleksanaa @Lucus16 @yorickvP @NixOSInfra