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

hydra-unstable: 2020-06-23 -> 2020-07-28 #93945

Merged
merged 1 commit into from Jul 30, 2020
Merged

hydra-unstable: 2020-06-23 -> 2020-07-28 #93945

merged 1 commit into from Jul 30, 2020

Conversation

ivan
Copy link
Member

@ivan ivan commented Jul 27, 2020

Motivation for this change

This fixes hydra-unstable on master, which currently does not build.

This adds a nixHydra because Hydra does not build with nixStable or nixUnstable: NixOS/hydra#794

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from edolstra July 27, 2020 02:53
@edolstra
Copy link
Member

I would prefer to remove hydra-unstable from Nixpkgs. I don't think we should maintain yet another version of Nix in Nixpkgs just for a pretty niche use case.

@ivan
Copy link
Member Author

ivan commented Jul 27, 2020 •
edited

I think it might be too late to leave it unfixed? The hydra module and release notes for 20.03 already strongly encouraged a migration to hydra-unstable, where hydra-unstable is

  hydra-unstable = callPackage ./common.nix {
    version = "2020-06-23";

I don't know how, or if, I could migrate back to an older Hydra.

Also, we could remove or alias nixHydra after NixOS/hydra#794 gets fixed.

cc @Ma27

@Ma27
Copy link
Member

Ma27 commented Jul 27, 2020

@edolstra we had this discussion in the past and while I understand your concerns, I'm still against it (for now at least) for the following reasons:

  • Hydra doesn't have any release model. This package in nixpkgs is maintained (by me currently), so people don't have to regularly check for new revisions from the git repo. Also, the implementation in nixpkgs has an upgrade-path for Hydra's schema change.

  • I know that hydra can (and probably should) be distributed using flakes. I actually use flakes for a while now and don't get me wrong, I really started to like this. However this feature isn't stable yet and I occasionally hit some problems when using nixUnstable by default, so the majority of average users can't really use this IMHO (I'd reconsider this as soon as there's a stable nix release including flakes (not hidden behind an experimental flag) which landed in a stable NixOS release).

I don't know how, or if, I could migrate back to an older Hydra.

That's not possible due to the DB schema changes from ~February 2020. But AFAIU Eelco prefers to distribute Hydra using flakes (as mentioned above, this is perfectly understandable, but IMHO not viable for the average users at the moment), not a downgrade to an older Hydra rev.

Also, we could remove or alias nixHydra after NixOS/hydra#794 gets fixed.

Actually, I'm not sure if we really want to do that: whenever nixUnstable gets bumped, Hydra tends to break. This is definetely not a complaint though, I really appreciate your work @edolstra (and I know how time-consuming OSS work can be), but for me as a maintainer of this (legacy) package, such a mechanism would be fairly helpful.

However there are two more things to discuss before merging:

  • I think it's better to expose nixHydra as hydra-unstable.passthru.nixHydra, using this helper-package for anything else must not be advertised (which is somehow the case by adding a top-level attribute) IMHO.
  • I really need to test if this causes conflicts between the Nix used by Hydra and the Nix used as daemon. @edolstra do you think this can potentially create issues?

@edolstra
Copy link
Member

Note that I'm not talking about removing hydra, just hydra-unstable. The number of people who need to run the bleeding edge is probably pretty small. And if they can't or don't want to use flakes, they can use a git checkout of hydra master. (That's what we used to do for hydra.nixos.org before flakes.)

@Ma27
Copy link
Member

Ma27 commented Jul 27, 2020

Well, but Hydra master is actually the one with flakes, right? And what would you choose in our case then? There are still no releases.

@ivan
hydra-unstable: 2020-06-23 -> 2020-07-28
0d7bdd6 
This fixes the build:

config.status: creating hydra-config.h
config.status: executing depfiles commands
config.status: executing libtool commands
config.status: executing executable-scripts commands
building
build flags: -j8 -l8 SHELL=/nix/store/c4wxsn4jays9j31y5z9f83nr2cp7l4pa-bash-4.4-p23/bin/bash
make  all-recursive
make[1]: Entering directory '/build/source'
Making all in src
make[2]: Entering directory '/build/source/src'
Making all in hydra-evaluator
make[3]: Entering directory '/build/source/src/hydra-evaluator'
g++ -DHAVE_CONFIG_H -I. -I../..    -std=c++17 -I/nix/store/2xhb4hlskn33pbyph36v4wxcan56dnrw-boehm-gc-8.0.4-dev/include -I/nix/store/5rjgsqjdm71cflfb68q7m771a1rqcsk3-nix-2.4pre20200719_a79b6dd-dev/include/nix -Wall -I ../libhydra -Wno-deprecated-declarations -g -O2 -std=c++17 -include nix/config.h -c -o hydra_evaluator-hydra-evaluator.o `test -f 'hydra-evaluator.cc' || echo './'`hydra-evaluator.cc
hydra-evaluator.cc:27:27: error: template argument 1 is invalid
   27 |     std::unique_ptr<Config> config;
      |                           ^
hydra-evaluator.cc:27:27: error: template argument 2 is invalid
hydra-evaluator.cc: In constructor 'Evaluator::Evaluator()':
hydra-evaluator.cc:61:56: error: base operand of '->' is not a pointer
   61 |         , maxEvals(std::max((size_t) 1, (size_t) config->getIntOption("max_concurrent_evals", 4)))
      |                                                        ^~
hydra-evaluator.cc:60:44: error: invalid user-defined conversion from 'std::_MakeUniq<Config>::__single_object' {aka 'std::unique_ptr<Config, std::default_delete<Config> >'} to 'int' [-fpermissive]
   60 |         : config(std::make_unique<::Config>())
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from /nix/store/3krz9s8ni3bqy4hy35ycmq8assrrb4f6-gcc-9.3.0/include/c++/9.3.0/memory:80,
                 from /nix/store/d97d0wixvlprz59z57maqj1pmda55r3b-libpqxx-6.4.5/include/pqxx/binarystring.hxx:17,
                 from /nix/store/d97d0wixvlprz59z57maqj1pmda55r3b-libpqxx-6.4.5/include/pqxx/binarystring:4,
                 from /nix/store/d97d0wixvlprz59z57maqj1pmda55r3b-libpqxx-6.4.5/include/pqxx/pqxx:3,
                 from ../libhydra/db.hh:3,
                 from hydra-evaluator.cc:1:
/nix/store/3krz9s8ni3bqy4hy35ycmq8assrrb4f6-gcc-9.3.0/include/c++/9.3.0/bits/unique_ptr.h:374:16: note: candidate is: 'std::unique_ptr<_Tp, _Dp>::operator bool() const [with _Tp = Config; _Dp = std::default_delete<Config>]' <near match>
  374 |       explicit operator bool() const noexcept
      |                ^~~~~~~~
/nix/store/3krz9s8ni3bqy4hy35ycmq8assrrb4f6-gcc-9.3.0/include/c++/9.3.0/bits/unique_ptr.h:374:16: note:   return type 'bool' of explicit conversion function cannot be converted to 'int' with a qualification conversion
make[3]: *** [Makefile:440: hydra_evaluator-hydra-evaluator.o] Error 1
make[3]: Leaving directory '/build/source/src/hydra-evaluator'
make[2]: *** [Makefile:360: all-recursive] Error 1
make[2]: Leaving directory '/build/source/src'
make[1]: *** [Makefile:414: all-recursive] Error 1
make[1]: Leaving directory '/build/source'
make: *** [Makefile:344: all] Error 2
builder for '/nix/store/g967cc3j6rc3nnpx2s4klpr03ig9zzyp-hydra-2020-06-23.drv' failed with exit code 2
@ivan
Copy link
Member Author

ivan commented Jul 28, 2020

@edolstra thank you very much for the fix in Hydra. I have updated this PR to just bump hydra-unstable.

@ivan ivan changed the title hydra-unstable: 2020-06-23 -> 2020-07-14 and add nixHydra hydra-unstable: 2020-06-23 -> 2020-07-28 Jul 29, 2020
@Ma27 Ma27 added the 9.needs: port to stable A PR needs a backport to the stable release. label Jul 30, 2020
@Ma27 Ma27 merged commit 8211932 into NixOS:master Jul 30, 2020
@Ma27
Copy link
Member

Ma27 commented Jul 30, 2020

Thanks! Ported to stable as 38516a2.

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Jul 30, 2020
@thefloweringash
Copy link
Member

Did this change the setup requirements for signing? I have a hydra running on 20.03 and I’m seeing failures when building on remote builders due to paths lacking signatures. See example build and hydra-queue-runner log.

The bump includes a change to how paths are copied, which looks related.

@Ma27
Copy link
Member

Ma27 commented Aug 3, 2020

Thanks for your report. Will take a look today or tomorrow.

@Ma27
Copy link
Member

Ma27 commented Aug 3, 2020

@thefloweringash is it possible that this is related to the NoCheckSigs-argument in the old behavior? If I understand this correctly, you could check if adding a single path like this to the store might fix the issue:

destStore->addToStore(info, from, NoRepair, NoCheckSigs);

For further reference you may want to look at the "old" behavior: https://github.com/NixOS/nix/blob/699fc89b394ef5347ff0508abe389e77a7cde09e/src/libstore/export-import.cc#L91

If this fixes your issue, please file a PR against NixOS/hydra (I'd also apply this as patch here if needed).

edolstra added a commit to NixOS/hydra that referenced this pull request Aug 4, 2020
@edolstra
Restore NoCheckSigs
77c33c1 
NixOS/nixpkgs#93945 (comment)
@edolstra
Copy link
Member

edolstra commented Aug 4, 2020

@Ma27 Thanks, I've added the NoCheckSigs back.

@Ma27
Copy link
Member

Ma27 commented Aug 4, 2020

Awesome, thanks! Will update the package later today :)

Ma27 added a commit that referenced this pull request Aug 4, 2020
@Ma27
hydra-unstable: 2020-07-28 -> 2020-08-04
ae9ab32 
Moving just one patch forward to fix importing store-paths from
`hydra-queue-runner`[1]. The other patches[2] require a newer
`pkgs.nixUnstable` and can't be used currently due to this.

[1] #93945 (comment)
[2] NixOS/hydra@77c33c1...4b58130
Ma27 added a commit that referenced this pull request Aug 4, 2020
@Ma27
hydra-unstable: 2020-07-28 -> 2020-08-04
b8151a4 
Moving just one patch forward to fix importing store-paths from
`hydra-queue-runner`[1]. The other patches[2] require a newer
`pkgs.nixUnstable` and can't be used currently due to this.

[1] #93945 (comment)
[2] NixOS/hydra@77c33c1...4b58130

(cherry picked from commit ae9ab32)
@Ma27
Copy link
Member

Ma27 commented Aug 4, 2020

@thefloweringash fixed on master in ae9ab32 and on release-20.03 in b8151a4.

@thefloweringash
Copy link
Member

Thanks for the quick fix!

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

@Ma27 Ma27 Ma27 approved these changes

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
8.has: package (new) 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ivan @edolstra @Ma27 @thefloweringash