Skip to content

Conversation

@piperswe
Copy link
Contributor

@piperswe piperswe commented Oct 13, 2021

Motivation for this change

These are the trivial changes from #141448.

nix build 'github:piperswe/nixpkgs/piper/add-sparc64#pkgsCross.sparc64-linux.hello'
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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.

@piperswe
Copy link
Contributor Author

@r-burns here's the trivial changes from the other pr!

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 13, 2021
lib = import ./lib;

systems = lib.systems.supported.hydra;
systems = lib.platforms.all;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the only part I'm not sure about, because I don't use flakes. does this just mean that you're allowed to evaluate nixpkgs on all platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, well I guess I don't understand why this would ever be limited in the first place. So platforms.all seems sensible

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

If a flakes person can confirm this is OK, this PR looks good to me. Or we can just punt on this, because I AFIAK this the thing the about flakes in general that's being debated right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm testing this through my personal github:piperswe/nixfiles flake on my personal Hydra instance, so I need flake support for this arch for those Hydra jobs to evaluate. I suppose I could add a non-flake jobset for these arches, but that is an added level of complexity that seems unnecessary if we can just export all platforms.

Copy link
Member

Choose a reason for hiding this comment

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

OK then we should not change this for now.

Copy link
Member

Choose a reason for hiding this comment

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

I'm 90% sure that this should not be changed. systems is like the system field of derivations (= the system/localSystem argument to nixpkgs) meaning the platform the derivation is built on. Consequently lib.systems.supported.hydra is Tier 3 and above, meaning all platforms for which a native stdenv is available.

For a jobset system can't be sparc64-linux atm anyways, as a native stdenv is not available, but rather pkgsCross needs to be used or crossSystem passed (which is not possible with flakes however as far as I know).

Copy link
Member

Choose a reason for hiding this comment

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

This change doesn't seem necessary given that sparc64-linux is only built using pkgsCross as @sternenseemann said.

The main thing you want to check is to see if nix flakes check keeps working after the change. It might also cause more/slower evaluation to have all the systems available.

@r-burns
Copy link
Contributor

r-burns commented Oct 13, 2021

This looks great to me, I was able to build the cross bootstrap tools and cross-compile some basic packages. Once this is merged the cross-trunk job will start building cross bootstrap tools for this platform so we'll have the cross stdenv in the cache.

cc @NixOS/exotic-platform-maintainers

Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Hi! It's always nice to see new platforms being added. I've reviewed the changes in lib (I'm not an expert on the other parts of Nixpkgs being changed here), and I have some changes to request.

  • Please maintain the alphabetical sorting in doubles.nix.

  • When adding new architectures, we should add them to doubles.nix for all operating systems that support that architecture. FreeBSD, OpenBSD, and NetBSD all appear to support sparc64.

  • Please add a filterDoubles line in doubles.nix for sparc64.

  • Going forward, I think platform examples contain the operating system name, because we support multiple operating systems (and hopefully increasingly so in future). So I think "sparc64-linux" would be a better name than "sparc64".

@siraben
Copy link
Member

siraben commented Oct 13, 2021

Cross-compiling GNU Hello on darwin works (pkgsCross.sparc64.hello) LGTM if @alyssais's comments are addressed.

@piperswe piperswe requested a review from alyssais October 13, 2021 23:59
@alyssais
Copy link
Member

Great work on the doubles, thanks! I'm still hoping we can get some clarity on the flakes thing — setting it to the tier3 platforms only happened recently, and I'd like to understand why that was done (which required that list to be created in the first place) instead of the much simpler solution of platforms.all.

@piperswe
Copy link
Contributor Author

Great work on the doubles, thanks! I'm still hoping we can get some clarity on the flakes thing — setting it to the tier3 platforms only happened recently, and I'd like to understand why that was done (which required that list to be created in the first place) instead of the much simpler solution of platforms.all.

Ah, I didn't notice that was a recent change. @zimbatm is there a reason why the flake doesn't export every platform?

@aaronjanse
Copy link
Member

Thank you @piperswe for making this PR! I'm excited to see sparc64 support in Nixpkgs

@alyssais
Copy link
Member

This is GTG apart from the flake.nix change, right? Could we just drop that for now, merge this, and have a separate PR for that change where it might be easier to get the attention of flakes experts?

@sternenseemann
Copy link
Member

Bumping: Can we get this rebased and the flake change dropped?

@Mindavi Mindavi added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Apr 13, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 2, 2022
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

10 participants