Skip to content

Improve release-cross tests#21414

Merged
Ericson2314 merged 4 commits intoNixOS:masterfrom
Ericson2314:release-cross
Dec 29, 2016
Merged

Improve release-cross tests#21414
Ericson2314 merged 4 commits intoNixOS:masterfrom
Ericson2314:release-cross

Conversation

@Ericson2314
Copy link
Member

Ahead of #21388 or #21268, it would be nice to test cross-compiling better. This PR cleans up the existing tests, and adds some new ones. Those new ones fail, but that's OK: they won't hold up any channel, and they'll remind us to merge one of the two PRs above, which fixes the new tests.

@DavidEGrayson What do you think? I'd add you as a reviewer on the side panel, but It won't let me. If you like this I'll go ahead and merge it.

@mention-bot
Copy link

@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @domenkozar and @rbvermaa to be potential reviewers.

@Ericson2314 Ericson2314 changed the title Release cross Improve release-cross tests Dec 25, 2016
@Ericson2314 Ericson2314 self-assigned this Dec 25, 2016
@Ericson2314 Ericson2314 added this to the 17.03 milestone Dec 25, 2016
@Ericson2314 Ericson2314 added 6.topic: cross-compilation Building packages on a different platform than they will be used on 0.kind: enhancement Add something new or improve an existing system. 8.has: clean-up This PR removes packages or removes other cruft labels Dec 25, 2016
Copy link
Contributor

@DavidEGrayson DavidEGrayson left a comment

Choose a reason for hiding this comment

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

The first commit was a little hard to review so I only glanced over it. The rest looks good, though I did point out one nit.

Copy link
Contributor

Choose a reason for hiding this comment

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

The expression in parentheses is really just a verbose way of writing testEqual.

Copy link
Member Author

@Ericson2314 Ericson2314 Dec 29, 2016

Choose a reason for hiding this comment

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

Oh hah! A few refactors in and I totally missed that worthless eta expansion. Thanks!

These derivations do not care about the target platform, and thus should
not be affected by cross-compiling. Currently, these tests *fail*, but they
will be fixed soon by a latter PR. The release-cross job doesn't block a
channel, so this should be no problem.
@Ericson2314 Ericson2314 merged commit 614ae8f into NixOS:master Dec 29, 2016
@Ericson2314 Ericson2314 deleted the release-cross branch December 29, 2016 17:59
@Ericson2314
Copy link
Member Author

http://hydra.nixos.org/eval/1317573 The good news is I didn't break any existing jobs, the bad news is that my assert ...; trues are not considered jobs.

@DavidEGrayson
Copy link
Contributor

Ah, I guess it has to be assert ...; someTrivialJob. But I don't really know how Hydra works.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Dec 29, 2016

Those assertions should be failing, so I figured Hydra would have no idea whether there is a derivation behind them. Maybe it has some magic, but in any event I don't want the job to dissapear if the assert isn't thrown. I'll try something like if .... then "passing-job" runCommand {} "touch $out" else runCommand "failing-job" {} "false"

@dezgeg
Copy link
Contributor

dezgeg commented Dec 29, 2016

FWIW they do show up in http://hydra.nixos.org/jobset/nixpkgs/cross-trunk#tabs-errors. But yes, you do need some sort of dummy derivations so they show up like the others.

@Ericson2314
Copy link
Member Author

Oh, that's a good start. It's weird that that tab is there with all the evaluations, rather than per-evaluation.

@DavidEGrayson
Copy link
Contributor

DavidEGrayson commented Jan 4, 2017

From this PR:

Converting to a string (drv path) before checking equality is probably a
good idea lest there be some irrelevant pass-through debug attrs that
cause false negatives.

Equality comparison of derivations seems to be special and different from equality comparison of a normal set. When you make the changes necessary to get those tests to pass, it proves that a derivation with a crossDrv inside it (for foosys and foolibc) can be equal to a derivation without a crossDrv. So I bet that debugging attributes also don't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement Add something new or improve an existing system. 6.topic: cross-compilation Building packages on a different platform than they will be used on 8.has: clean-up This PR removes packages or removes other cruft

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants