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
Revert "nixos: doc: implement related packages in the manual" #33006
Conversation
I'm pretty sure this is not a problem made by my PR. I did your thing on
my `nixos/related-packages` branch and it succeeds:
```
HOME=/homeless-shelter NIX_PATH=nixpkgs=$(pwd) nix-instantiate ./nixos/release.nix -A manual --option restrict-eval true --option build-timeout 1800 --argstr system x86_64-linux --show-trace
warning: you did not specify ‘--add-root’; the result might be removed by the garbage collector
/nix/store/64srhwmmx4anfsj4k38shv9g8f0cjk7c-nixos-manual.drv
```
Does reverting really help? If so I'm gonna be puzzled, since I didn't
touch anything of essence in `xen` expressions.
|
It did help, maybe merge master in to your branch and try again? |
It did help, maybe merge master in to your branch and try again?
Yes, cherry-picking those patches back does break it on master.
Bisecting shows that it breaks on the last commit in my branch.
Moreover, removing `relatedPackages =` lines from `xen-dom0.nix` in that
commit fixes it. Which means the problem is with evaluating `xen`
expressions inside `genRelatedPackages`. `genRelatedPackages` accesses
`package.name` and `package.meta` while generating the docbook blob and
that fails.
Is in no longer allowed to touch those attrs on `master`? Why does it
mention `aarch64-linux`? Is the manual built for the `aarch64-linux` on
`x86_64-linux` or what?
|
Okay, so the culprit for this error on master is
08b8bc2. Reverting it and applying my
original changes from `nixos/related-packages` branch makes everything
work as expected.
The thing is, of course, that its not
08b8bc2 that is the wrong here, nor are
my patches, the problem is that 08b8bc2
makes `-A manual` evaluate manual for `aarch64-linux` and that fails
because accessing `${package.name}` and `${package.meta}` on a platform
unsupported by a given package fails. I think the latter is a bug
somewhere in `stdenv.mkDerivation`/`check-meta.nix` or related which
ought to be fixed by someone who knows how those things work in detail.
I'm feeling fairly incompetent looking at that code.
|
The issue as I understand it is the manual must build for all supported systems in the release.nix:
the manual is built from a config:
and buildFromConfig builds it for every supportedSystem:
where forAllSystems iterates over each supportedSystem. Perhaps one way to handle this in the doc generation code is to handle an unsupported system and skip the package, but I don't know the best way. Out of my league too. Maybe @vcunat could provide some guidance on this. |
It is all as you say, but why
Perhaps one way to handle this in the doc generation code is to handle
an unsupported system and skip the package, but I don't know the best
way.
Why can't `stdenv` just allow access to `.name` and `.meta` even on
unsupported systems? I can, of course, `filter` out unsupported packages
from related packages in `genRelatedPackages` if someone hints at how to
do that without triggering `check-meta.nix`, but then it would build
differing manuals on different systems, which could cause a lot of
confusion. I'd prefer `stdenv` to be fixed instead.
|
Why can't `stdenv` just allow access to `.name` and `.meta` even on
unsupported systems?
I think I accidentally made that happen. Please see my
nixos/related-packages-v3 branch. It's not ideal, but it works. Btw, I
can guess why it works, but I'm not really sure (I just read the related
code and thought "hm... what if I hack `addPassthru` a little bit" and,
surprise, it worked).
Now the manual builds as expected, but those three patches have a
side-effect of allowing access not only to `.name` and `.meta`
attributes but to any non-output attribute of a derivation without
checking the meta. I think this should've been the semantics from the
start but maybe some tests somewhere depend on the previous way of doing
things.
In short, please play with it too.
|
See #33057. Its a relevant to that bug part of nixos/related-packages-v3, but with a bit simpler and cleaner implementation.
|
Reverts #32424
I'm not sure why exactly, but this is causing problems after merging that ofborg didn't catch prior to merging:
@oxij I like the work and didn't want to revert it, but it wasn't clear to me how to fix it. Can you try again?