Revert "nixos: doc: implement related packages in the manual"#33006
Merged
Revert "nixos: doc: implement related packages in the manual"#33006
Conversation
GrahamcOfBorg
added
10.rebuild-darwin: 0
Member
|
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.
|
Member
Author
|
It did help, maybe merge master in to your branch and try again? |
Member
|
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?
|
Member
|
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.
|
Member
Author
|
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. |
Member
|
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.
|
Member
|
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.
|
3 tasks
Member
|
See #33057. Its a relevant to that bug part of nixos/related-packages-v3, but with a bit simpler and cleaner implementation.
|
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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?