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
texinfo: 6.5 -> 6.7; fix cross compiling #95910
Conversation
@GrahamcOfBorg build texinfo |
@thefloweringash according to git blame, you added the interactive cross-build check. Maybe you can shed some light on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this change causes over 500 rebuilds, do you mind targeting the staging branch
git rebase --onto=staging HEAD~1
git push .. ... --force
then change the base branch in the github PR from master
-> staging
See https://nixos.org/nixpkgs/manual/#submitting-changes-staging-branch for more details on staging branch
$ nix-env -f /home/jon/.cache/nixpkgs-review/pr-95910/nixpkgs -qaP --xml --out-path --show-trace --meta
28975 packages updated:
@GrahamcOfBorg build texinfo |
@jonringer Done. |
Non-interactive build mode is needed when using it for bootstrapping (otherwise it causes dependency loops, similarly to bash and stdenv) and `doCheckByDefault` nixpkgs feature.
So do not remove it, please.
|
if it is just used for bootstrapping, maybe the cross + non-interactive case won't occur very often in practice. That cross patch looks very innocuous. Maybe we should try to always apply it? This is a mass rebuild already. |
@oxij This change does not remove interactive mode, it just removes the flag that determines that cross-build patches are only applied on interactive mode. Now, they are applied on both, my question is, why where they separated in the first place in regards to cross building? |
It was a while ago, but as far as I can tell only the interactive case was broken and it avoided a mass rebuild according to the tags on #76598. I don’t know what changed to make this now required for the non-interactive case, but it also fails for me on master. Testing as of 6dd60c6
|
Is there any way to get this patch improved to move it along? I know it's a bit last minute, but I'd really hate to miss the 20.09 release window with this ... |
This broke darwin stdenv and is blocking staging-next (and branch-off thereby) https://nix-cache.s3.amazonaws.com/log/lnkwicmp12bcy9fhdillhkwh2xm9px8b-texinfo-6.7.drv |
Proposed revert #97218. |
This PR contains two commits, the cross-compile fixes and the version bump, the later of which seems to introduce this regression. |
A fix is preferred, and otherwise its a revert. I was under the impression it was the interactive change, but did not give it much thought. I can't test it myself either, like most of us... |
Or postpone this whole PR after 20.09? I can't see why it needs to be in there as a last-minute change. (I can't test Darwin either.) |
It's basically a partial revert of PR #95910. I chose a temporar-ish solution, maximizing likelihood of fixing this while minimizing rebuilds. (20.09 process is being blocked)
makeinfo seems right to fail when input encoding is not declared and is not UTF-8. texinfo was updated in #95910.
makeinfo seems right to fail when input encoding is not declared and is not UTF-8. texinfo was updated in NixOS#95910. (cherry picked from commit 19f7f15)
Motivation for this change
The current derivation does not correctly cross-build when building in non-interactive mode.
Looking at the derivation, I don't see a reason for this distinction at all, hence this PR removes it.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)