Can this PR be reviewed? We have found that we need this for environments that have a PATH with a space.

Thanks!

I wonder if this change is the reason why tests are failing

I wonder if this change is the reason why tests are failing

Interesting:

/bin/sh: 1: protoc: Permission denied

Does this mean that make verify in the github action was previously finding a different protoc (that properly has the x bit set)?

I'm guessing that we also need:

diff --git a/Makefile b/Makefile
index 3cc72405..f189183b 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,7 @@ ifeq ($(shell uname | tr A-Z a-z), linux)
        curl -L https://github.com/protocolbuffers/protobuf/releases/download/v>
 endif
        unzip bin/protoc.zip -d bin/protoc
+       chmod +x bin/protoc
        rm bin/protoc.zip
 
 bin/protoc-gen-go:

For the second CI failure:

#89 0.177 /bin/sh: uname: not found
#89 0.181 /bin/sh: uname: not found

I'm not sure if that's the actual underlying error, but I do see that in the Github Action log. I'm assuming it's from the various calls to $(shell uname) in the Makefile.

But offhand, I can't think of why adding the quotes to the PATH would make uname suddenly be unable to be found...?

Oh, actually, I see this in the log output, too:

#89 9.400 env: can't execute 'sh': No such file or directory

Odd.

I'm guessing that we also need:

I added a commit to this PR that added the chmod +x .... Could you approve the github actions again to see if that problem at least got fixed?

I haven't had time to dig into the 2nd failure yet, but I offhand wonder if that failed because protoc previously didn't run...?

Oops -- had the wrong chmod path in there. Fixed.

Sorry to bug -- could you authorize the github actions again? Thanks!

It took some digging, but I tracked down what was causing CI to fail, and re-evaluated the solution of this PR. I have pushed a new commit with a full explanation. Here's the commit message:

Makefile: Do not export SHELL

The intent for exporting the SHELL variable is to ensure that all
Makefile target comands are run with the amended PATH value that is
set via the "export PATH := ..." directive.

However, exporting PATH value is all that is necessary to ensure that
all target commands run with the amended PATH. It is not necessary to
also export a SHELL command that explicitly sets the PATH value.

Specifically: setting SHELL to use "env" to set the PATH environment
variable gets difficult if the PATH includes whitespace. make's
assignment right hand side parsing does not treat quotes as delimiters
of tokens. For example, the following directive in a Makefile results
in the FOO variable having 3 tokens, not 2:

FOO := bar "baz yow"

Therefore, achieving the overall goal of amending the PATH for all
Makefile target commands -- even if the PATH includes whitespace -- is
best achieved simply by not exporting the SHELL.

Signed-off-by: Ethan Dieterich ethandieterich@gmail.com
Signed-off-by: Jeff Squyres jsquyres@cisco.com

Congrats on the 4000th PR. 🎉