All of lore.kernel.org
 help / color / mirror / Atom feed
From: "D. Ben Knoble" <ben.knoble+github@gmail.com>
To: git@vger.kernel.org
Cc: "D. Ben Knoble" <ben.knoble+github@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Noah Pendleton <noah.pendleton@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Thranur Andul <thranur@gmail.com>,
	Michael Grosser <grosser.michael@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v2 0/3] Support :(optional) filepaths
Date: Sun, 28 Sep 2025 17:29:13 -0400	[thread overview]
Message-ID: <cover.1759094936.git.ben.knoble+github@gmail.com> (raw)
In-Reply-To: <20250501214057.371711-1-gitster@pobox.com>

Notes:
- Based on commit 2da08f2c3d (parseopt: values of pathname type can be
  prefixed with :(optional), 2024-10-14) (broken-out/wip/optional-path)
- Rebased on v2.51.0
- I'm least sure of the 3rd patch and am happy to drop it in support of
  the first 2. I think it might be better to (a) integrate :(optional)
  support as pathspec magic and (b) use pathspec magic in parse-options
  when getting filenames. But I'm not sure, and this has other
  ramifications I'm not prepared to deal with. (For example: `git grep
  path <file>… :(optional)non-existent` could pretend like
  `non-existent` was never given?)
- The parsing is not exactly a "clean API," but I wasn't sure how to
  make it cleaner :)

Changes in v2:
- Only check for missing files, not empty files
- Move a test change to the appropriate commit
- Document optional magic in options in gitcli(1)

This series adds support for optional filepaths in config and
parse-options, which supports use-cases such as missing commit templates
or blame.ignoreRevsFile values without erroring.

v1: https://lore.kernel.org/git/20250501214057.371711-1-gitster@pobox.com/

Junio C Hamano (3):
  t7500: make each piece more independent
  config: values of pathname type can be prefixed with :(optional)
  parseopt: values of pathname type can be prefixed with :(optional)

 Documentation/config.adoc                 |  4 ++-
 Documentation/gitcli.adoc                 | 14 +++++++++
 config.c                                  | 16 +++++++++--
 parse-options.c                           | 31 +++++++++++++-------
 t/t7500-commit-template-squash-signoff.sh | 35 +++++++++++++++++------
 wrapper.c                                 | 13 +++++++++
 wrapper.h                                 |  4 ++-
 7 files changed, 94 insertions(+), 23 deletions(-)

Diff-intervalle contre v1 :
1:  82d283c626 ! 1:  63b2b24d42 t7500: make each piece more independent
    @@ Commit message
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
      ## t/t7500-commit-template-squash-signoff.sh ##
    +@@ t/t7500-commit-template-squash-signoff.sh: commit_msg_is ()
    + 	(
    + 		GIT_EDITOR="echo hello >\"\$1\"" &&
    + 		export GIT_EDITOR &&
    +-		test_must_fail git commit
    ++		test_must_fail git commit --allow-empty
    + 	)
    + '
    + 
     @@ t/t7500-commit-template-squash-signoff.sh: commit_msg_is ()
      TEMPLATE="$PWD"/template
      
2:  dbafaff13b ! 2:  5c97f580a9 config: values of pathname type can be prefixed with :(optional)
    @@ Commit message
         pathname wants to signal such an optional file, it can be marked by
         prepending ":(optional)" in front of it.  Such a setting that is
         marked optional would avoid getting the command barf for a missing
    -    file, as an optional configuration setting that names a missing or
    -    an empty file is not even seen.
    +    file, as an optional configuration setting that names a missing
    +    file is not even seen.
     
         cf. <xmqq5ywehb69.fsf@gitster.g>
     
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
    - ## Documentation/config.txt ##
    -@@ Documentation/config.txt: compiled without runtime prefix support, the compiled-in prefix will be
    +
    + ## Notes ##
    +    The 2nd paragraph in this commit is wrapped strangely
    +
    +    I've kept the strange wrapping length for now, but can reflow it if
    +    desired.
    +
    + ## Documentation/config.adoc ##
    +@@ Documentation/config.adoc: compiled without runtime prefix support, the compiled-in prefix will be
      substituted instead. In the unlikely event that a literal path needs to
      be specified that should _not_ be expanded, it needs to be prefixed by
      `./`, like so: `./%(prefix)/bin`.
     -
     ++
     +If prefixed with `:(optional)`, the configuration variable is treated
    -+as if it does not exist, if the named path does not exist or names an
    -+empty file.
    ++as if it does not exist, if the named path does not exist.
      
      Variables
      ~~~~~~~~~
    @@ config.c: int git_config_string(char **dest, const char *var, const char *value)
     +	if (!path)
      		die(_("failed to expand user dir in: '%s'"), value);
     +
    -+	if (is_optional && is_empty_or_missing_file(path)) {
    ++	if (is_optional && is_missing_file(path)) {
     +		free(path);
     +		return 0;
     +	}
    @@ t/t7500-commit-template-squash-signoff.sh: commit_msg_is ()
      # From now on we'll use a template file that exists.
      TEMPLATE="$PWD"/template
      
    +
    + ## wrapper.c ##
    +@@ wrapper.c: int xgethostname(char *buf, size_t len)
    + 	return ret;
    + }
    + 
    ++int is_missing_file(const char *filename)
    ++{
    ++	struct stat st;
    ++
    ++	if (stat(filename, &st) < 0) {
    ++		if (errno == ENOENT)
    ++			return 1;
    ++		die_errno(_("could not stat %s"), filename);
    ++	}
    ++
    ++	return 0;
    ++}
    ++
    + int is_empty_or_missing_file(const char *filename)
    + {
    + 	struct stat st;
    +
    + ## wrapper.h ##
    +@@ wrapper.h: void write_file_buf(const char *path, const char *buf, size_t len);
    + __attribute__((format (printf, 2, 3)))
    + void write_file(const char *path, const char *fmt, ...);
    + 
    +-/* Return 1 if the file is empty or does not exists, 0 otherwise. */
    ++/* Return 1 if the file does not exist, 0 otherwise. */
    ++int is_missing_file(const char *filename);
    ++/* Return 1 if the file is empty or does not exist, 0 otherwise. */
    + int is_empty_or_missing_file(const char *filename);
    + 
    + enum fsync_action {
3:  2da08f2c3d ! 3:  5f7057c236 parseopt: values of pathname type can be prefixed with :(optional)
    @@ Commit message
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
    + ## Documentation/gitcli.adoc ##
    +@@ Documentation/gitcli.adoc: $ git describe --abbrev=10 HEAD  # correct
    + $ git describe --abbrev 10 HEAD  # NOT WHAT YOU MEANT
    + ----------------------------
    + 
    ++
    ++Magic filename options
    ++~~~~~~~~~~~~~~~~~~~~~~
    ++Options that take a filename allow a prefix `:(optional)`. For example:
    ++
    ++----------------------------
    ++git commit -F :(optional)COMMIT_EDITMSG
    ++# if COMMIT_EDITMSG does not exist, equivalent to
    ++git commit
    ++----------------------------
    ++
    ++Like with configuration values, if the named file is missing Git behaves as if
    ++the option was not given at all. See "Values" in linkgit:git-config[1].
    ++
    + NOTES ON FREQUENTLY CONFUSED OPTIONS
    + ------------------------------------
    + 
    +
      ## parse-options.c ##
     @@ parse-options.c: static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
      {
    - 	const char *s, *arg;
    + 	const char *arg;
      	const int unset = flags & OPT_UNSET;
     -	int err;
      
    @@ t/t7500-commit-template-squash-signoff.sh: commit_msg_is ()
      test_expect_success 'nonexistent template file in config should return error' '
      	test_config commit.template "$PWD"/notexist &&
      	(
    - 		GIT_EDITOR="echo hello >\"\$1\"" &&
    - 		export GIT_EDITOR &&
    --		test_must_fail git commit
    -+		test_must_fail git commit --allow-empty
    - 	)
    - '
    - 

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.48.1


  parent reply	other threads:[~2025-09-28 21:30 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07 20:27 [PATCH 0/1] blame: Skip missing ignore-revs file Noah Pendleton
2021-08-07 20:58 ` Junio C Hamano
2021-08-07 21:34   ` Noah Pendleton
2021-08-08  5:43     ` Junio C Hamano
2021-08-08 17:50       ` Junio C Hamano
2021-08-08 18:21         ` Noah Pendleton
2021-08-09 15:47           ` Junio C Hamano
2024-10-14 20:44             ` [PATCH 0/3] specifying a file that can optionally exist Junio C Hamano
2024-10-14 20:44               ` [PATCH 1/3] t7500: make each piece more independent Junio C Hamano
2024-10-14 20:44               ` [PATCH 2/3] config: values of pathname type can be prefixed with :(optional) Junio C Hamano
2024-10-14 20:44               ` [PATCH 3/3] parseopt: " Junio C Hamano
2025-05-01 21:40             ` [PATCH 0/3] specifying a file that can optionally exist Junio C Hamano
2025-05-01 21:40               ` [PATCH 1/3] t7500: make each piece more independent Junio C Hamano
2025-05-01 21:40               ` [PATCH 2/3] config: values of pathname type can be prefixed with :(optional) Junio C Hamano
2025-05-02  8:52                 ` Patrick Steinhardt
2025-05-02 14:28                   ` Phillip Wood
2025-05-02 20:05                   ` Junio C Hamano
2025-05-01 21:40               ` [PATCH 3/3] parseopt: " Junio C Hamano
2025-09-28 21:29               ` D. Ben Knoble [this message]
2025-09-28 21:29                 ` [PATCH v2 1/3] t7500: make each piece more independent D. Ben Knoble
2025-09-28 21:29                 ` [PATCH v2 2/3] config: values of pathname type can be prefixed with :(optional) D. Ben Knoble
2025-09-30 15:26                   ` Phillip Wood
2025-10-06 19:00                     ` Junio C Hamano
2025-10-06 19:59                       ` Junio C Hamano
2025-10-06 20:21                         ` Junio C Hamano
2025-10-06 20:22                           ` Junio C Hamano
2025-10-07 12:24                           ` Kristoffer Haugsbakk
2025-10-07 17:04                             ` Junio C Hamano
2025-11-02 16:20                     ` D. Ben Knoble
2025-09-28 21:29                 ` [PATCH v2 3/3] parseopt: " D. Ben Knoble
2025-09-30 15:26                   ` Phillip Wood
2025-11-02 16:20                     ` D. Ben Knoble
2025-11-03  0:10                       ` Eric Sunshine
2025-11-04 18:22                         ` D. Ben Knoble
2025-09-28 22:40                 ` [PATCH v2 0/3] Support :(optional) filepaths Junio C Hamano
2025-09-29 16:42                   ` Ben Knoble
2025-10-20  9:40                 ` [PATCH] t7500: fix tests with absolute path following ":(optional)" on Windows Johannes Sixt
2025-10-20 13:43                   ` Ben Knoble
2025-10-20 17:32                     ` Johannes Sixt
2025-10-20 18:06                       ` Junio C Hamano
2025-10-20 20:27                       ` D. Ben Knoble
2025-10-20 20:27                         ` D. Ben Knoble
2025-10-20 17:39                     ` Eric Sunshine
2025-10-20 16:17                   ` Junio C Hamano
2025-10-20 17:24                     ` Johannes Sixt
2022-03-04  9:51   ` [PATCH 0/1] blame: Skip missing ignore-revs file Thranur Andul
2021-08-08 17:48 ` [PATCH v2] blame: add config `blame.ignoreRevsFileIsOptional` Noah Pendleton
  -- strict thread matches above, loose matches on Subject: below --
2025-04-25 18:41 Feature request: automatically read .git-blame-ignore-revs or allow global optional config Michael Grosser
2025-04-25 19:54 ` Eric Sunshine
2025-05-01 18:00   ` D. Ben Knoble
2025-05-01 18:28     ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1759094936.git.ben.knoble+github@gmail.com \
    --to=ben.knoble+github@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=grosser.michael@gmail.com \
    --cc=noah.pendleton@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    --cc=sunshine@sunshineco.com \
    --cc=thranur@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.