build: shell script portability fixes (TMPDIR, ar path, shellcheck warnings)

[randomizedcoder]

Shell Script Portability Improvements

Summary

This PR improves portability of PCP's build scripts for non-FHS systems (like NixOS, Guix) and sandboxed build environments. It also fixes shellcheck warnings to improve script quality.

New Pull request in response to
#2444 (review)

Changes

1. Portable ar fallback in configure

The configure script had a hardcoded fallback to /usr/bin/ar, which doesn't exist on systems that don't follow the Filesystem Hierarchy Standard. Changed to just ar which will be found via PATH.

-test -n "$AR" || AR="/usr/bin/ar"
+test -n "$AR" || AR="ar"

Please note that the configure script has a very large number of shellcheck exceptions, so this pull request did NOT attempt to resolve these.

2. Use ${TMPDIR:-/tmp} instead of /var/tmp

Several build scripts hardcode /var/tmp for temporary files. This fails in sandboxed build environments (like Nix) where /var/tmp is not writable. The fix uses ${TMPDIR:-/tmp} which:

• Uses $TMPDIR when set (required for sandboxed builds)
• Falls back to /tmp on traditional systems
• Is POSIX-compliant

Affected files:

• src/pmdas/bind2/mk.rewrite
• src/pmdas/jbd2/mk.rewrite
• src/pmdas/linux/mk.rewrite
• src/pmdas/linux_proc/mk.rewrite
• src/bashrc/getargs
• src/libpcp/src/check-errorcodes
• src/libpcp3/src/check-errorcodes
• src/libpcp/doc/mk.cgraph
• src/pmlogcompress/check-optimize

3. Shellcheck warning fixes (all POSIX sh compatible)

Fix	Issue	Description
Trap quoting	SC2064	Use single quotes so $tmp expands at signal time, not trap definition time
Test portability	SC2166	Replace [ -a ] with [ ] && [ ] (more portable)
Command substitution	SC2046	Quote $() to prevent word splitting
Variable init	SC2154	Initialize sts before trap to avoid false warning

Testing

• Verified build completes successfully
• Tested with Nix flake build and NixOS VM test
• All shellcheck warnings resolved in modified files

[randomizedcoder]

This Pull Request relates to #2444

[kmcdonell]

Hmm @randomizedcoder we could be in for a bit of a bumpy ride here ...

1. The configure change won't fly. Our configure is generated from configure.ac and this fragment of setting AR comes from autoconf ... you'll need to either set AR=...; export AR before you run configure else propose a change for configure.ac that changes the value of AR conditionally if $AR (as set by the autoconf-generated preamble) is not found. This line dnl check if the tar program is available in configure.ac is the start of a block that does something similar for gnutar on macOS, but you'll need a test -x rather than a test -z.
2. The tmp=${TMPDIR:-/tmp}/foo-$$ ones are OK ... we don't use $TMPDIR in the code at all so no nasty aliasing surprises and moving from /var/tmp to /tmp in the default (common) case is also probably the right thing to do these days (the original rationale for using /var/tmp based on common disk and root filesystem sizes went away decades ago)
3. The "checkshell" ones I don't like. I can't see these actually fixing a problem, some I think are wrong, e.g. trap "...\$foo" is not a problem and even trap "...$foo is OK if $foo is assigned a value before the trap is declared and is not changed before the trap is executed. Similarly rewriting test foo -a bar as test foo && test bar serves no useful purpose. We're getting into the realm of religious arguments here, and I'd assert if it ain't broken then don't fix it.

I'd suggest reworking the AR change and drop the shellcheck changes.
Once this is merged you can cycle back to the nix PR #2440.
Then if you wish we can revisit the shellcheck issues if any real problems remain after that.

[kmcdonell]

Ping @randomizedcoder ... any progress here?

[kmcdonell]

G'day @randomizedcoder,
As of commit bad6e53 2. above is done all across the PCP code base (not just the ones you've spotted below src).
Unless you want to engage in a conversation to convince me about 3., I think this is now down to 1. Would you like me to "fix" this in configure.ac?
Hopefully we can move this PR to closure, then you'll need to rebase PR #2444.

[randomizedcoder]

G'day mate!

I was just started to get back to this, but it looks like you fixed it up already! Nice work.

I'm a pretty big fan of shellcheck, and don't see a good reason not to follow it. John Carmack of Doom game fame says lean in on code checkers and I complete agree.

How can I help?

Thanks,
Dave

[randomizedcoder]

I think we can close this pull request.

I will review the nix one again after this nice fixes you just applied.

[randomizedcoder]

Created a pull request for "ar"
#2479

[kmcdonell]

OK, @randomizedcoder, I'm going to close this one.

Just one last shot on code checkers ... I've been coding in C, sh and troff for more than 50 years. And my experience with code checkers is mixed.

• gcc is pretty good, but just gets it wrong sometimes, which is why the PCP code is littered with "pander to gcc" comments
• coverity (serious static analysis) gets it right just often enough to warrant making the effort, but about half the "issues" there turn out to be false positives
• mandoc -T lint (troff checker) appears to have been written by people who simply do not understand the troff language
• I have no experience with shellcheck (but I will do some research and learning).

Thanks.