-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
groff: 1.22.4 -> 1.23.0 #241870
groff: 1.22.4 -> 1.23.0 #241870
Conversation
@ofborg build groff groff.passthru.tests |
Do you see it by chance? Or should I retest on clean |
Thanks for trying out this PR, @trofi. At first glance the failed tests might be related to bug #61561: [grotty] remove 'sgr' device control command. Does this seem plausible to you? |
Tested on clean
|
Yep. Thanks for working on this. |
Thank you for testing this PR in a clean I raised this issue on the groff mailing list and the helpful folks noticed that NixOS disables groff colors by default by outputting From this I see several paths forward:
Another recommendation from the groff mailing list was to ensure hyperlinks are enabled for terminals by having (For details see this mail thread) Before making more changes to this PR I'd like to be sure that the general direction is right for NixOS, i.e. enable SGR or maintain legacy output format (see above).
|
Good find! Looks like it was set up in 111b5eb |
FWIW, I'd go with upstream recommendation unless there's a good reason not to. |
Does it make
|
☝️ The changes in this PR now have the @trofi can you build another ℹ️ This mail thread from the groff mailing list goes a bit into the history of where changes to Glad to you share my view on the upstream recommendation, @toonn. ❓ What other Nix folks or channels should be made aware of this proposed change, so people can chime in with their expertise? ❓ Are the changes in Additionally I'd like to introduce a |
Still fails like in the comment from 30 minutes ago: #241870 (comment) |
Thanks for the update @trofi! Seems like GitHub did not show me your recent comment until after I posted mine. Let me look into this. |
Would it have to change configureFlags? Won't that cause a rebuild of groff and therefore almost anything with man pages? It seems like groff should be respecting locale settings. |
I’m running |
$ nix build 'github:NixOS/nixpkgs?ref=pull/241870/head#groff'
$ ./result/bin/groff --version
GNU groff version 1.23.0
Copyright (C) 2022 Free Software Foundation, Inc.
GNU groff comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of groff and its subprograms
under the terms of the GNU General Public License.
For more information about these matters, see the file
named COPYING.
called subprograms:
GNU troff (groff) version 1.23.0
GNU grops (groff) version 1.23.0 |
Can you reproduce trofi's failure for |
It builds for me on aarch64-darwin. $ nix build 'github:NixOS/nixpkgs?ref=pull/241870/head#man-db'
$ ./result/bin/mandb --help
Usage: mandb [OPTION...] [MANPATH]
-c, --create create dbs from scratch, rather than updating
-C, --config-file=FILE use this user configuration file
-d, --debug emit debugging messages
-f, --filename=FILENAME update just the entry for this filename
-p, --no-purge don't purge obsolete entries from the dbs
-q, --quiet work quietly, except for 'bogus' warning
-s, --no-straycats don't look for or add stray cats to the dbs
-t, --test check manual pages for correctness
-u, --user-db produce user databases only
-?, --help give this help list
--usage give a short usage message
Mandatory or optional arguments to long options are also mandatory or optional
for any corresponding short options. |
Today's output for
|
I get a similar failure on aarch64-linux. |
It builds if I build it interactively using |
I can confirm that @reckenrode can you share more details on the failure on x86_64 and aarch64 linux or is it exactly the |
It’s the same error.
|
Thanks for the helpful comment and question, @toonn, much appreciated! It made me rethink about my idea. To answer your questions:
There might be other ways, but it's the "easiest" I've found so far.
I'm afraid yes, so that approach is very likely highly undesirable.
There is a whole email thread over on the groff mailing list that does into the detail about selecting the papersize (on the fly), which helped me understand a bit of the history of why things are the way they are. I'm uncertain whether the locale is a good reference to derive the preferred papersize from. Linux systems seem to offer I'll keep digging for another way to customize the groff default paper format from within the nixpkgs package. |
This would explain why man-db builds on Darwin. It doesn’t do any checks.
|
Great find @reckenrode, I went ahead and locally removed the Darwin man-db test failures
|
macOS sets EDIT: This is a lie, I was setting this with HM. Excuse me for the confusion. |
@toonn, unfortunately that does not seem to be the case for me. Checking
Do you have any reference about |
ℹ️ The man-db test failures have been raised upstream. |
Enable tests on Darwin
Kindly requesting re-review now that upstream has provided patches |
☝️ /cc @trofi, @wegank, @toonn, @reckenrode |
@ofborg eval |
I've been building this for the last two days again. No eval errors yet but only halfway there. I'm not sure why but many Python packages seem to be taking ages to build. |
I was able to build groff on both x86_64-darwin and aarch64-darwin. I didn’t run nixpkgs-review due to the (presumably) large number of rebuilds. |
That's good news, thanks @reckenrode. Do you know if any actions need to be taken for this PR in order to address the ofborg-eval check and if yes which actions? |
AFAICT that's unrelated. My build still hasn't finished but I'll trust reckenrode's. |
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.
Previous message should've been an approval rather than a comment.
I was going to say. 😅 I think the go-modules stuff is fallout from #242905. |
Thanks everyone for your help on this, I'm very excited for this to have been merged! 😃👍 |
Description of changes
Update groff to 1.23.0
✅
In order toenableHtml
on Darwin #241869 is needed as it fixes thepsutils
build on the platform.ℹ️ Notice that groffer has been removed from the upstream repo (see upstream commit
1d2df80a933d
for details).Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)