Skip to content
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

build: sort.py: use case-sensitive sorting #6070

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Oct 27, 2023

To match how things are sorted elsewhere, such as with noblacklist /
whitelist lines (vertically) in profiles and in
ci/check/profiles/sort-disable-programs.sh and src/etc-cleanup/main.c.

This makes the order in private-etc always be groups (@group), then
uppercase paths, then lowercase paths. Example from
etc/profile-m-z/softmaker-common.profile:

private-etc @tls-ca,SoftMaker,fstab

Note that this does not affect a significant amount of profiles; most
changes are in private-bin / private-lib lines and in private-etc
lines for newer profiles that do not use groups. This is partly due to
commit 5d0822c ("private-etc: big profile changes", 2023-02-05)
replacing X11 with @x11 in private-etc lines and then commit
0f996ea ("private-etc: groups modified", 2023-02-05) removing
Trolltech.conf from private-etc lines and using case-sensitive
sorting in them.

Relates to #5610.

@kmk3 kmk3 requested review from glitsj16 and rusty-snake October 27, 2023 19:38
To match how things are sorted elsewhere, such as with `noblacklist` /
`whitelist` lines (vertically) in profiles and in
ci/check/profiles/sort-disable-programs.sh and src/etc-cleanup/main.c.

This makes the order in `private-etc` always be groups (`@group`), then
uppercase paths, then lowercase paths.  Example from
etc/profile-m-z/softmaker-common.profile:

    private-etc @tls-ca,SoftMaker,fstab

Note that this does not affect a significant amount of profiles; most
changes are in `private-bin` / `private-lib` lines and in `private-etc`
lines for newer profiles that do not use groups.  This is partly due to
commit 5d0822c ("private-etc: big profile changes", 2023-02-05)
replacing `X11` with `@x11` in `private-etc` lines and then commit
0f996ea ("private-etc: groups modified", 2023-02-05) removing
`Trolltech.conf` from `private-etc` lines and using case-sensitive
sorting in them.

Relates to netblue30#5610.
Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes more sense sorting things this way. Nice job!

@netblue30 netblue30 merged commit 03e75e5 into netblue30:master Nov 24, 2023
5 checks passed
@netblue30
Copy link
Owner

All in!

@kmk3 kmk3 deleted the sort-py-csort branch November 25, 2023 12:48
kmk3 added a commit to kmk3/firejail that referenced this pull request Nov 25, 2023
It was disabled on commit df6ea88 ("merges, disable sort.py in profile
checks temporarely, two more private-etc profiles", 2023-02-14).

Currently all profiles are sorted and there are no ongoing `private-etc`
changes, so it should be safe to re-enable.

Note that the script is useful to catch sorting issues not only in
`private-etc` but also in other commands, such as `seccomp`[1] [2].

This is a follow-up to netblue30#6070.

Relates to netblue30#5610.

[1] netblue30#6066 (comment)
[2] netblue30#6067 (comment)
kmk3 added a commit to kmk3/firejail that referenced this pull request Nov 25, 2023
It was disabled on commit df6ea88 ("merges, disable sort.py in profile
checks temporarely, two more private-etc profiles", 2023-02-14).

Currently all profiles are sorted and there are no ongoing `private-etc`
changes, so it should be safe to re-enable.

Note that the script is useful to catch sorting issues not only in
`private-etc` but also in other commands, such as `seccomp`[1] [2].

This is a follow-up to netblue30#6070.

Relates to netblue30#5610.

[1] netblue30#6066 (comment)
[2] netblue30#6067 (comment)
@kmk3 kmk3 mentioned this pull request Nov 25, 2023
kmk3 added a commit to kmk3/firejail that referenced this pull request Nov 26, 2023
It was disabled on commit df6ea88 ("merges, disable sort.py in profile
checks temporarely, two more private-etc profiles", 2023-02-14).

Currently all profiles are sorted and there are no ongoing `private-etc`
changes, so it should be safe to re-enable.

Note that the script is useful to catch sorting issues not only in
`private-etc` but also in other commands, such as `seccomp`[1] [2].

This is a follow-up to netblue30#6070.

Relates to netblue30#5610.

[1] netblue30#6066 (comment)
[2] netblue30#6067 (comment)
kmk3 added a commit to kmk3/firejail that referenced this pull request Mar 26, 2024
That is, make "X11" lowercase so that the order of the includes in the
disable- section remain the same when sorted with `LC_ALL=C`, as is the
case for most of the other sections.  That is also likely to be the
default in text editors (such as in vim on Arch), so this should make
the disable- section more consistent and easier to sort when editing the
profile.

Also, keep the old include as a redirect to the new one for now to avoid
breakage.

Commands used to search and replace:

    git mv etc/inc/disable-X11.inc etc/inc/disable-x11.inc
    git grep -Ilz 'disable-X11' -- etc | xargs -0 \
      perl -pi -e 's/disable-X11/disable-x11/'

Relates to netblue30#4462 netblue30#4854 netblue30#6070 netblue30#6289.

This is a follow-up to netblue30#6286.
kmk3 added a commit that referenced this pull request Mar 27, 2024
That is, make "X11" lowercase so that the order of the includes in the
disable- section remain the same when sorted with `LC_ALL=C`, as is the
case for most of the other sections.  That is also likely to be the
default in text editors (such as in vim on Arch), so this should make
the disable- section more consistent and easier to sort when editing the
profile.

Also, keep the old include as a redirect to the new one for now to avoid
breakage.

Commands used to search and replace:

    git mv etc/inc/disable-X11.inc etc/inc/disable-x11.inc
    git grep -Ilz 'disable-X11' -- etc | xargs -0 \
      perl -pi -e 's/disable-X11/disable-x11/'

Relates to #4462 #4854 #6070 #6289.

This is a follow-up to #6286.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

3 participants