Skip to content

scripts: use end-of-options marker in cd, mkdir, mv, sha256sum commands#22150

Closed
vszakats wants to merge 52 commits into
curl:masterfrom
vszakats:secarg
Closed

scripts: use end-of-options marker in cd, mkdir, mv, sha256sum commands#22150
vszakats wants to merge 52 commits into
curl:masterfrom
vszakats:secarg

Conversation

@vszakats

@vszakats vszakats commented Jun 23, 2026

Copy link
Copy Markdown
Member

Where missing. To avoid passing an option by accident.

End-of-option marker (--) is not POSIX, but most of these scripts are
internal and/or CI-focused, where this is fine. maketgz and
verify-release are meant for public use, and I asses this is fine too,
but let us know if this causes issues in real world envs.

Also:

  • maketgz: pass args with :? to rm -rf, where missing.
    Cannot cause an actual issue with current code.
  • verified cp, rm instances too, but none were affected.
  • tests/cmake/test.sh: replace $PWD with $(pwd) for sturdiness.
  • appveyor.sh: replace $PWD with $(pwd) for sturdiness.

Assisted-by: Dan Fandrich
Follow-up to 6aab1dc #19450


  • filter out cases where it's not necessary based on wider context.
    esp Makefile.am, maketgz and verify-release.
  • double-check if -- is necessary/useful in the cd -- "$(dirname "$0")" command.
  • split off unrelated "also" fixes into separate PR. → autotools: minor fixes and improvements #22154
  • fix dist fallout due to dropping MANFILES.

move to #22154:

@dfandrich

dfandrich commented Jun 23, 2026 via email

Copy link
Copy Markdown
Contributor

@vszakats

Copy link
Copy Markdown
Member Author

NB -- to end arguments is not POSIX, so this will cause breakage on some systems.

I suspect it may be an issue for Makefile.am cases in practice.
(curl-config is not affected). Some of the Makefile.am ones
could be removed after closer inspection, or maybe even all of
them to avoid issues – I'll review tomorrow. FTR there is already
one use in master, via 6aab1dc,
with no reports so far.

Then there is maketgz/verify-release, which I'll also review,
though I surmise they may be fine with this.

The others seem fine, as they are CI/developer/internal.

@vszakats vszakats marked this pull request as draft June 24, 2026 01:07
@vszakats vszakats changed the title scripts: pass -- before passing variable arguments scripts: use end-of-options marker in cd, cp, mkdir, mv, rm commands Jun 24, 2026
@vszakats

Copy link
Copy Markdown
Member Author

After review, there is just one case left in Makefile.am,
in the same rule that already had one. I feel this is OK.

The rest is much reduced, in most cases to cd, and
a couple of others.

I'll move out not stritcly related updates to separate
PR(s). (some $?s may be overkill, but if not causing
issues I'd keep them for safety.)

vszakats added 3 commits June 24, 2026 13:09
This reverts commit b27979a.

I'm not grokking how this works, but this simplification did not.

TODO: read up on:
https://www.gnu.org/software/automake/manual/html_node/Clean.html
DISTCLEANFILES are supposed to cover stuff created by configure,
and these files are not like that.

https://www.gnu.org/software/automake/manual/html_node/Clean.html
@vszakats

Copy link
Copy Markdown
Member Author

Turns out the remaining Makefile.am update is unnecessary as well.

@vszakats vszakats changed the title scripts: use end-of-options marker in cd, cp, mkdir, mv, rm commands scripts: use end-of-options marker in cd, cp, mkdir, mv, sha256sum commands Jun 24, 2026
@vszakats vszakats marked this pull request as ready for review June 24, 2026 13:35
@vszakats vszakats changed the title scripts: use end-of-options marker in cd, cp, mkdir, mv, sha256sum commands scripts: use end-of-options marker in cd, mkdir, mv, sha256sum commands Jun 24, 2026
@vszakats vszakats closed this in ccf19d5 Jun 25, 2026
vszakats added a commit that referenced this pull request Jun 25, 2026
- Makefile.am: add root `test-quiet` target that maps to
  `tests/quiet-test`, for completeness.
  Ref: #22098

- tests/Makefile.am: drop unused remains of `MANFILES`.
  Follow-up to fa3f889 #17463
  Also replace the `distclean` target to delete `CLEANFILES`, otherwise
  the CI dist job fails. (I'm not grokking why)

- tests/Makefile.am: sync up `tests` target with CMake, to pass `TFLAGS`
  `-a`.
  Follow-up to 904e7ec #19347

- tests/certs/Makefile.am: fix to clean generated certs via `CLEANFILES`
  to comply with autotools documentation, which says the previously used
  variable `DISTCLEANFILES` is meant for files created by `./configure`.
  Ref: https://www.gnu.org/software/automake/manual/html_node/Clean.html
  Follow-up to 44341e7 #16824

- tests/http/Makefile.am: add `./` prefix to glob passed to `rm -rf` on
  clean, to avoid deleting unintented files, or pass unintended options.
  Follow-up to 6711582 #10349

Cherry-picked from #22102
Cherry-picked from #22150

Closes #22154
@vszakats vszakats deleted the secarg branch June 25, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants