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: strip whitespace in profiles #6556

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Nov 29, 2024

build: sort.py: strip trailing whitespace in all lines

Currently the output is mangled if the last item on the line contains
trailing whitespace and is moved when sorting.

So remove trailing whitespace in all lines (that is, not just in lines
containing supported commands).

Leave leading whitespace as is for now since it could potentially be
used for indentation.

Before:

$ printf '# hello world  \nprivate-bin a,b  \nprivate-bin b,a  \nprivate-bin  a,b\n' \
  >foo.profile
$ ./contrib/sort.py -n foo.profile | tr ' ' .
sort.py:.checking.1.profile(s)...
foo.profile:3:-private-bin.b,a..
foo.profile:3:+private-bin.a..,b

After:

$ printf '# hello world  \nprivate-bin a,b  \nprivate-bin b,a  \n' \
  >foo.profile
$ ./contrib/sort.py -n foo.profile | tr ' ' .
sort.py:.checking.1.profile(s)...
foo.profile:1:-#.hello.world..
foo.profile:1:+#.hello.world
foo.profile:2:-private-bin.a,b..
foo.profile:2:+private-bin.a,b
foo.profile:3:-private-bin.b,a..
foo.profile:3:+private-bin.a,b

build: sort.py: strip whitespace in commands

Currently whitespace is left as is within an entry.

In a protocol entry, if there is whitespace between the command and
its argument or around an item, the item in question is dropped from the
output.

Changes:

  • protocol: Strip all whitespace in the argument
  • Other commands: Strip leading/trailing whitespace around each item,
    including any extra whitespace between a command and its argument

Note: Whitespace characters inside paths are left as is, as some paths
(such as Foo Bar may contain spaces.

Before:

$ printf 'private-bin a,b\nprivate-bin  a,b\nprivate-bin  b,a\nprivate-bin  C,A  B\nprotocol  unix,net\nprotocol  inet,unix\n' \
  >foo.profile
$ ./contrib/sort.py -n foo.profile
sort.py: checking 1 profile(s)...
foo.profile:5:-protocol  unix,net
foo.profile:5:+protocol
foo.profile:6:-protocol  inet,unix
foo.profile:6:+protocol unix

After:

$ printf 'private-bin a,b\nprivate-bin  a,b\nprivate-bin  b,a\nprivate-bin  C,A  B\nprotocol  unix,net\nprotocol  inet,unix\n' \
  >foo.profile
$ ./contrib/sort.py -n foo.profile
sort.py: checking 1 profile(s)...
foo.profile:2:-private-bin  a,b
foo.profile:2:+private-bin a,b
foo.profile:3:-private-bin  b,a
foo.profile:3:+private-bin a,b
foo.profile:4:-private-bin  C,A  B
foo.profile:4:+private-bin A  B,C
foo.profile:5:-protocol  unix,net
foo.profile:5:+protocol unix
foo.profile:6:-protocol  inet,unix
foo.profile:6:+protocol unix,inet

@kmk3 kmk3 requested a review from rusty-snake November 29, 2024 07:30
@kmk3 kmk3 added the enhancement New feature request label Dec 2, 2024
@kmk3
Copy link
Collaborator Author

kmk3 commented Dec 3, 2024

Leading whitespace could maybe be left as is, in case indentation is used for
whatever reason.

@rusty-snake Any thoughts on this PR?

I did not notice any performance impact, by the way.

@kmk3 kmk3 force-pushed the sort-py-strip-ws branch from f7d7196 to 0f23429 Compare December 5, 2024 07:42
kmk3 added 2 commits December 5, 2024 04:43
Set `fixed_line` to `line` and only use the latter when needed.

This makes it easier to modify `fixed_line` multiple times.
Rename `line` to `original_line` to make it less likely to accidentally
read from/write to it instead of the fixed line.

Rename `fixed_line` to `line` to make the code shorter since it is now
referenced much more often (up to 3 times in the same line of code) than
the original line.

See also commit aa17ca5 ("sort.py: rename protocols to
original_protocols", 2022-10-17) / PR netblue30#5429.
@kmk3 kmk3 force-pushed the sort-py-strip-ws branch from 0f23429 to 01fde78 Compare December 5, 2024 07:52
kmk3 added 2 commits December 5, 2024 04:53
Currently the output is mangled if the last item on the line contains
trailing whitespace and is moved when sorting.

So remove trailing whitespace in all lines (that is, not just in lines
containing supported commands).

Leave leading whitespace as is for now since it could potentially be
used for indentation.

Before:

    $ printf '# hello world  \nprivate-bin a,b  \nprivate-bin b,a  \nprivate-bin  a,b\n' \
      >foo.profile
    $ ./contrib/sort.py -n foo.profile | tr ' ' .
    sort.py:.checking.1.profile(s)...
    foo.profile:3:-private-bin.b,a..
    foo.profile:3:+private-bin.a..,b

After:

    $ printf '# hello world  \nprivate-bin a,b  \nprivate-bin b,a  \n' \
      >foo.profile
    $ ./contrib/sort.py -n foo.profile | tr ' ' .
    sort.py:.checking.1.profile(s)...
    foo.profile:1:-#.hello.world..
    foo.profile:1:+#.hello.world
    foo.profile:2:-private-bin.a,b..
    foo.profile:2:+private-bin.a,b
    foo.profile:3:-private-bin.b,a..
    foo.profile:3:+private-bin.a,b
Currently whitespace is left as is within an entry.

In a `protocol` entry, if there is whitespace between the command and
its argument or around an item, the item in question is dropped from the
output.

Changes:

* `protocol`: Strip all whitespace in the argument
* Other commands: Strip leading/trailing whitespace around each item,
  including any extra whitespace between a command and its argument

Note: Whitespace characters inside paths are left as is, as some paths
(such as `Foo Bar` may contain spaces.

Before:

    $ printf 'private-bin a,b\nprivate-bin  a,b\nprivate-bin  b,a\nprivate-bin  C,A  B\nprotocol  unix,net\nprotocol  inet,unix\n' \
      >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:5:-protocol  unix,net
    foo.profile:5:+protocol
    foo.profile:6:-protocol  inet,unix
    foo.profile:6:+protocol unix

After:

    $ printf 'private-bin a,b\nprivate-bin  a,b\nprivate-bin  b,a\nprivate-bin  C,A  B\nprotocol  unix,net\nprotocol  inet,unix\n' \
      >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:2:-private-bin  a,b
    foo.profile:2:+private-bin a,b
    foo.profile:3:-private-bin  b,a
    foo.profile:3:+private-bin a,b
    foo.profile:4:-private-bin  C,A  B
    foo.profile:4:+private-bin A  B,C
    foo.profile:5:-protocol  unix,net
    foo.profile:5:+protocol unix
    foo.profile:6:-protocol  inet,unix
    foo.profile:6:+protocol unix,inet
@kmk3 kmk3 force-pushed the sort-py-strip-ws branch from 01fde78 to 08e5f81 Compare December 5, 2024 07:54
@kmk3 kmk3 merged commit 3826645 into netblue30:master Dec 5, 2024
5 checks passed
@kmk3 kmk3 deleted the sort-py-strip-ws branch December 5, 2024 08:01
kmk3 added a commit that referenced this pull request Dec 9, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Dec 28, 2024
Changes:

* Strip whitespace at the beginning
* Strip whitespace at the end
* Ensure exactly one newline at the end
* Strip extraneous newlines

Also, for clarity print the git diff in the sort.py ci job, since the
specific lines changed are not printed by the sort.py script in this
case (as whitespace is fixed in the entire profile at once).

Command used to search and replace:

    ./contrib/sort.py etc/inc/*.inc etc/profile*/*.profile

This is a follow-up to netblue30#6556.
kmk3 added a commit to kmk3/firejail that referenced this pull request Dec 28, 2024
Changes:

* Strip whitespace at the beginning
* Strip whitespace at the end
* Ensure exactly one newline at the end
* Strip extraneous newlines

Also, for clarity print the git diff in the sort.py ci job, since the
specific lines changed are not printed by the sort.py script in this
case (as whitespace is fixed in the entire profile at once).

Command used to search and replace:

    ./contrib/sort.py etc/inc/*.inc etc/profile*/*.profile

This is a follow-up to netblue30#6556.

Update contrib/sort.py
kmk3 added a commit that referenced this pull request Dec 28, 2024
Changes:

* Strip whitespace at the beginning
* Strip whitespace at the end
* Ensure exactly one newline at the end
* Strip extraneous newlines

Also, for clarity print the git diff in the sort.py ci job, since the
specific lines changed are not printed by the sort.py script in this
case (as whitespace is fixed in the entire profile at once).

Command used to search and replace:

    ./contrib/sort.py etc/inc/*.inc etc/profile*/*.profile

This is a follow-up to #6556.

Update contrib/sort.py
kmk3 added a commit to kmk3/firejail that referenced this pull request Dec 29, 2024
To make it clearer when only whitespace was fixed on a given line.

Before:

    $ printf 'private-bin a,b \n' >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:1:-private-bin a,b
    foo.profile:1:+private-bin a,b

After:

    $ printf 'private-bin a,b \n' >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:1:-'private-bin a,b '
    foo.profile:1:+'private-bin a,b'

See commit 53ff8e0 ("build: sort.py: strip trailing whitespace in all
lines", 2024-11-26) / PR netblue30#6556.
kmk3 added a commit that referenced this pull request Dec 29, 2024
To make it clearer when only whitespace was fixed on a given line.

Before:

    $ printf 'private-bin a,b \n' >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:1:-private-bin a,b
    foo.profile:1:+private-bin a,b

After:

    $ printf 'private-bin a,b \n' >foo.profile
    $ ./contrib/sort.py -n foo.profile
    sort.py: checking 1 profile(s)...
    foo.profile:1:-'private-bin a,b '
    foo.profile:1:+'private-bin a,b'

See commit 53ff8e0 ("build: sort.py: strip trailing whitespace in all
lines", 2024-11-26) / PR #6556.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

2 participants