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

Makefile: use xargs -0 consistently #6706

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

whitslack
Copy link
Collaborator

@whitslack whitslack commented Sep 21, 2023

In its default mode, xargs interprets any whitespace as a delimiter, and it interprets single and double quotes and backslashes specially. This can play havoc when fed by tools that don't escape these special characters in their output.

Fortunately, xargs (at least the GNU and macOS versions of it) has a -0 option that changes the behavior so that only NUL characters act as delimiters, and no other characters have any special meaning. Use this option consistently to avoid any nasty surprises.

Note that git-ls-files has a -z option that causes it to terminate filenames on output with a NUL character instead of a newline, and grep has a -z option that causes it to operate on NUL-terminated records rather than on lines. Use these options together with xargs -0.

Note: This commit corrects an oversight introduced in 9d94687.

@@ -385,7 +385,7 @@ $(GRPC_GEN)&: cln-grpc/proto/node.proto cln-grpc/proto/primitives.proto
$(PYTHON) -m grpc_tools.protoc -I cln-grpc/proto cln-grpc/proto/node.proto --python_out=$(GRPC_PATH)/ --grpc_python_out=$(GRPC_PATH)/ --experimental_allow_proto3_optional
$(PYTHON) -m grpc_tools.protoc -I cln-grpc/proto cln-grpc/proto/primitives.proto --python_out=$(GRPC_PATH)/ --experimental_allow_proto3_optional
find $(GRPC_DIR)/ -type f -name "*.py" -print0 | xargs -0 sed -i'.bak' -e 's/^import \(.*\)_pb2 as .*__pb2/from pyln.grpc import \1_pb2 as \1__pb2/g'
find $(GRPC_DIR)/ -type f -name "*.py.bak" -print0 | xargs rm -f
find $(GRPC_DIR)/ -type f -name "*.py.bak" -print0 | xargs -0 rm -f
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the actual fix. The other changes are defensive.

@whitslack
Copy link
Collaborator Author

whitslack commented Sep 21, 2023

Just kidding about 91d48c7. GNU sed doesn't recognize the next arg after -i as the backup suffix because GNU getopt only recognizes optional option arguments if they're in the same word as the option flag.

@rustyrussell
Copy link
Contributor

Nice change. You kind of blamed the wrong commit (it just added -f to rm, before that was the same issue), but doesn't matter.

In its default mode, `xargs` interprets any whitespace as a delimiter,
and it interprets single and double quotes and backslashes specially.
This can play havoc when fed by tools that don't escape these special
characters in their output.

Fortunately, `xargs` (at least the GNU version of it) has a `-0` option
that changes the behavior so that only NUL characters act as delimiters,
and no other characters have any special meaning. Use this option
consistently to avoid any nasty surprises.

Note that `git-ls-files` has a `-z` option that causes it to terminate
filenames on output with a NUL character instead of a newline, and
`grep` has a `-z` option that causes it to operate on NUL-terminated
records rather than on lines. Use these options together with
`xargs -0`.

Note: This commit corrects an oversight introduced in
9d94687.

Changelog-None
@rustyrussell
Copy link
Contributor

Rebased on latest master to avoid CI brokenness.

@whitslack
Copy link
Collaborator Author

You kind of blamed the wrong commit

@rustyrussell: How is that? 9d94687 changed find … -delete to find … -print0 | xargs rm -f.

@rustyrussell rustyrussell merged commit 15b1ccb into ElementsProject:master Sep 27, 2023
35 checks passed
@whitslack whitslack deleted the xargs-0 branch September 27, 2023 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants