Page MenuHomeFreeBSD

sort: Only build FreeBSD-specific ALTMON_x stuff when ATLMON_1 is defined
ClosedPublic

Authored by imp on Dec 1 2023, 7:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 4, 5:00 PM
Unknown Object (File)
Sat, May 4, 4:11 PM
Unknown Object (File)
Sat, May 4, 1:16 PM
Unknown Object (File)
Sun, Apr 28, 3:34 AM
Unknown Object (File)
Fri, Apr 26, 2:30 AM
Unknown Object (File)
Tue, Apr 23, 3:54 PM
Unknown Object (File)
Feb 11 2024, 9:45 AM
Unknown Object (File)
Jan 13 2024, 5:22 AM
Subscribers

Details

Summary

On MacOS, we bootstrap sort. Since ALTMON_* are not defined there, the
build blows up. Since we don't need this feature for the FreeBSD build
process, and since we won't use it unless we actually install the NL
files that have this data in it, just #ifdef it out for now. In the
extremely unlikely event that the FreeBSD bootstrap/build process grows
this dependency, we can evaluate the best solution then (which most
likely is going to be not depend on the local's month names).

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Dec 1 2023, 7:11 PM
imp created this revision.
jrtc27 added a subscriber: jrtc27.

Seems fine to me. Do we have tests for this case to ensure that we don't silently start compiling this code out even on FreeBSD if something gets screwed up?

This revision is now accepted and ready to land.Dec 1 2023, 7:16 PM

Seems ok to me. Please include a "Fixes:" tag in the commit message so that it's easy to find this at MFC time.

I plan to push this EOD if I don't hear anything (since it is a build and CI breakage).
However, if there's something better in the wings, I'll back off.. and if something better comes along after my deadline, I'm totally cool deferring to that as well. (unless it is ifdef apple :).

Seems fine to me. Do we have tests for this case to ensure that we don't silently start compiling this code out even on FreeBSD if something gets screwed up?

I believe he added tests for it in the original commit, so I think we're good.

usr.bin/sort/bwstring.c
141

Actually, we should set *.alt = NULL here instead, or ifdef out the references in bws_month_score() as well.

NULL out .alt out of safety concerns if ALTMON_x is undefined.

This revision now requires review to proceed.Dec 1 2023, 7:25 PM
imp marked an inline comment as done.Dec 1 2023, 7:26 PM
imp added inline comments.
usr.bin/sort/bwstring.c
141

I think they will tend to be NULL anyway... but an explicit setting wouldn't hurt in case my thoughts are wrong :)

In D42868#977777, @imp wrote:

I plan to push this EOD if I don't hear anything (since it is a build and CI breakage).
However, if there's something better in the wings, I'll back off.. and if something better comes along after my deadline, I'm totally cool deferring to that as well. (unless it is ifdef apple :).

๐Ÿ˜‚ note taken. It builds!: https://github.com/jlduran/freebsd-src/actions/runs/7064234126/job/19231839355

This revision is now accepted and ready to land.Dec 1 2023, 7:28 PM
markj added inline comments.
usr.bin/sort/bwstring.c
141

I don't see how they would be NULL. sort_malloc() calls malloc(), not calloc().

Thanks. If you want, I can take care of MFC'ing it along with D42847.

This revision was automatically updated to reflect the committed changes.
imp marked an inline comment as done.

Thanks. If you want, I can take care of MFC'ing it along with D42847.

@imp done.