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

PULL_REQUEST_TEMPLATE: Encourage maintainers to test cross-building #96568

Closed
wants to merge 1 commit into from

Conversation

kampka
Copy link
Contributor

@kampka kampka commented Aug 28, 2020

Motivation for this change

At the current state of things, if feels like cross-building packages (eg. nixpkgs.crossSystem = { system = "aarch64-linux"; config = "aarch64-unknown-linux-gnu"; }) break quite often or is quite often already broken.
This change hopes to encourage package maintainers to test for cross-building of packages as part of testing pull requests.

At the current state of things, if feels like cross-building packages (eg. ` nixpkgs.crossSystem = { system = "aarch64-linux"; config = "aarch64-unknown-linux-gnu"; }`) break quite often or is quite often already broken.
This change hopes to encourage package maintainers to test for cross-building of packages as part of testing pull requests.
@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

Cross-platform is too wide, there are too many different archs to compile to. We also do not have any cross CI (I don't count the cross Hydra set for master, its not used for gating). We first need to define what archs to support (Tier 2 if I am correct, see NixOS/rfcs#46).

If you need stability I suggest sticking to a stable release.

@kampka
Copy link
Contributor Author

kampka commented Aug 29, 2020

If you need stability I suggest sticking to a stable release.

The issue is that stable releases usually don't build cross-platform without patching over issues through overlays and the like, hence this PR.

@kampka
Copy link
Contributor Author

kampka commented Aug 29, 2020

Cross-platform is too wide, there are too many different archs to compile to. We also do not have any cross CI (I don't count the cross Hydra set for master, its not used for gating).

The issue I encounter the most when dealing with cross-build issues is that dependencies are not correctly defined, eg. using buildInputs instead of nativeBuildInputs or using perl instead of buildPerl. These issues should show when building against basically any foreign platform. We don't need to have an air-tight solution here to remove a lot of issues from the current state of things.

@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

We don't need to have an air-tight solution here to remove a lot of issues from the current state of things.

That I agree with.

The issue I encounter the most when dealing with cross-build issues is that dependencies are not correctly defined, eg. using buildInputs instead of nativeBuildInputs

Here using strictDeps = true; throughout would help a lot. Basically one large PR setting strictDeps = true; for all packages that we can cross-compile already would probably go a long way for keeping them working. Maybe we should ask authors to set that attribute?

@kampka
Copy link
Contributor Author

kampka commented Aug 31, 2020

I am always for being more strict / precise when defining dependencies, but I would estimate that this roughly breaks a shitload of builds with not always obvious solutions, so I doubt a lot of maintainers are willing to go the extra mile if they don't have to.

On the plus side, it seems that @GrahamcOfBorg is willing to build crossPkgs if asked for it. Maybe it would be good if maintainers regularly checked for that when reviewing merge requests?

@FRidh
Copy link
Member

FRidh commented Aug 31, 2020

I am always for being more strict / precise when defining dependencies, but I would estimate that this roughly breaks a shitload of builds with not always obvious solutions, so I doubt a lot of maintainers are willing to go the extra mile if they don't have to.

Setting it in the stdenv, probably, but setting it for packages that currently cross-compile, clearly no. Obviously that is an effort.

Maybe it would be good if maintainers regularly checked for that when reviewing merge requests?

This is just shifting work to others.

I am closing this PR.

@FRidh FRidh closed this Aug 31, 2020
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

2 participants