{"thread":{"id":"61478","subject":"[PATCH] archive: make --add-virtual-file honor --prefix","startedAt":"2024-05-14T21:15:31Z","lastAt":"2024-06-26T19:06:09Z","messageCount":14,"participants":["Tom Scogland via GitGitGadget","Junio C Hamano","Tom Scogland","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"494748","messageId":"pull.1719.git.git.1715721327429.gitgitgadget@gmail.com","threadId":"61478","inReplyTo":null,"subject":"[PATCH] archive: make --add-virtual-file honor --prefix","fromName":"Tom Scogland via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-05-14T21:15:26Z","receivedAt":"2024-05-14T21:15:31Z","isPatch":true,"sender":{"key":"scogland1@llnl.gov","avatar":null},"body":"From: Tom Scogland <scogland1@llnl.gov>\n\nThe documentation for archive states:\n\n  The path of the file in the archive is built by concatenating the\n  value of the last `--prefix` moption (if any) before this\n  `--add-virtual-file` and <path>.\n\nThis matches the documentation for --add-file and the behavior works for\nthat option, but --prefix is ignored for --add-virtual-file.\n\nThis commit modifies archive.c to include the prefix in the path and\nadds a check into the existing add-virtual-file test to ensure that it\nhonors both the most recent prefix before the flag.\n\nIn looking for others with this issue, I found message\na143e25a70b44b82b4ee6fa3bb2bcda4@atlas-elektronik.com on the mailing\nlist, where Stefan proposed a basically identical patch to archive.c\nback in February, so the main addition here is the test along with that\npatch.\n\nSigned-off-by: Tom Scogland <scogland1@llnl.gov>\n---\n    archive: make --add-virtual-file honor --prefix\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1719%2Ftrws%2Fhonor-prefix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1719/trws/honor-prefix-v1\nPull-Request: https://github.com/git/git/pull/1719\n\n archive.c              | 11 +++++------\n t/t5003-archive-zip.sh |  8 ++++++--\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 5287fcdd8e0..90086a1708f 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -365,12 +365,11 @@ int write_archive_entries(struct archiver_args *args,\n \n \t\tput_be64(fake_oid.hash, i + 1);\n \n+\t\tstrbuf_reset(&path_in_archive);\n+\t\tif (info->base)\n+\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n+\t\tstrbuf_addstr(&path_in_archive, basename(path));\n \t\tif (!info->content) {\n-\t\t\tstrbuf_reset(&path_in_archive);\n-\t\t\tif (info->base)\n-\t\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n-\t\t\tstrbuf_addstr(&path_in_archive, basename(path));\n-\n \t\t\tstrbuf_reset(&content);\n \t\t\tif (strbuf_read_file(&content, path, info->stat.st_size) < 0)\n \t\t\t\terr = error_errno(_(\"cannot read '%s'\"), path);\n@@ -381,7 +380,7 @@ int write_archive_entries(struct archiver_args *args,\n \t\t\t\t\t\t  content.buf, content.len);\n \t\t} else {\n \t\t\terr = write_entry(args, &fake_oid,\n-\t\t\t\t\t  path, strlen(path),\n+\t\t\t\t\t  path_in_archive.buf, path_in_archive.len,\n \t\t\t\t\t  canon_mode(info->stat.st_mode),\n \t\t\t\t\t  info->content, info->stat.st_size);\n \t\t}\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex 961c6aac256..acc8bc4fcd6 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -218,14 +218,18 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '\n \tfi &&\n \tgit archive --format=zip >with_file_with_content.zip \\\n \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n-\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n+\t\t--add-virtual-file=hello:world \\\n+\t\t--prefix=subdir/ --add-virtual-file=hello:world \\\n+\t\t--prefix= $EMPTY_TREE &&\n \ttest_when_finished \"rm -rf tmp-unpack\" &&\n \tmkdir tmp-unpack && (\n \t\tcd tmp-unpack &&\n \t\t\"$GIT_UNZIP\" ../with_file_with_content.zip &&\n \t\ttest_path_is_file hello &&\n \t\ttest_path_is_file \"$PATHNAME\" &&\n-\t\ttest world = $(cat hello)\n+\t\ttest world = $(cat hello) &&\n+\t\ttest_path_is_file subdir/hello &&\n+\t\ttest world = $(cat subdir/hello)\n \t)\n '\n \n\nbase-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8\n-- \ngitgitgadget\n"},{"id":"494820","messageId":"xmqqcypn14ld.fsf@gitster.g","threadId":"61478","inReplyTo":"pull.1719.git.git.1715721327429.gitgitgadget@gmail.com","subject":"Re: [PATCH] archive: make --add-virtual-file honor --prefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-15T15:23:42Z","receivedAt":"2024-05-15T15:23:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tom Scogland via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Tom Scogland <scogland1@llnl.gov>\n>\n> The documentation for archive states:\n>\n>   The path of the file in the archive is built by concatenating the\n>   value of the last `--prefix` moption (if any) before this\n>   `--add-virtual-file` and <path>.\n>\n> This matches the documentation for --add-file and the behavior works for\n> that option, but --prefix is ignored for --add-virtual-file.\n\nThis first paragraph was a bit hard to parse for me.\n\nYou will contrast the quoted paragraph with another one you did not\nquote later in this paragraph, so it is more helpful to readers if\nyou instead said:\n\n    The documentatation for archive describes its\n    \"--add-virtual-file\" option like so:\n\n        ... excerpt from --add-virtual-file description ...\n\n    This description is the same as \"--add-file\", and \"--add-file\"\n    does behave the way as described.  \"--add-virtual-file\" however\n    ignores \"--prefix\".\n\n> This commit modifies archive.c to include the prefix in the path and\n> adds a check into the existing add-virtual-file test to ensure that it\n> honors both the most recent prefix before the flag.\n\nStyle: \"This comit modifies\" -> \"Modify\".\n\nAn obvious alternative fix is to update the documentation, which\nwould be a much safer thing to do, given that there may be existing\nscripts written during the two years since --add-virtual-file option\nwas introduced and has been behaving exactly this way.  They will\nall be broken big time once the command starts honoring the\n\"--prefix\" option.\n\n> In looking for others with this issue, I found message\n> a143e25a70b44b82b4ee6fa3bb2bcda4@atlas-elektronik.com on the mailing\n> list, where Stefan proposed a basically identical patch to archive.c\n> back in February, so the main addition here is the test along with that\n> patch.\n\nThis pargraph should come _after_ the three-dash lines below.\n\n> Signed-off-by: Tom Scogland <scogland1@llnl.gov>\n> ---\n>     archive: make --add-virtual-file honor --prefix\n\nThe implementation looked obvious, assuming that it is a good idea\nto change it (I've already talked about a safer alternative fix).\n\n> diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\n> index 961c6aac256..acc8bc4fcd6 100755\n> --- a/t/t5003-archive-zip.sh\n> +++ b/t/t5003-archive-zip.sh\n> @@ -218,14 +218,18 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '\n>  \tfi &&\n>  \tgit archive --format=zip >with_file_with_content.zip \\\n>  \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n> -\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n> +\t\t--add-virtual-file=hello:world \\\n> +\t\t--prefix=subdir/ --add-virtual-file=hello:world \\\n> +\t\t--prefix= $EMPTY_TREE &&\n\nInstead of reusing the exactly the same name and contents, use\nsomething different so that it is clear to the later test which of\nthe two \"--add-virtual-file\" options created these two paths in the\nunpacked directories.  I.e., create something like\n\n\t--prefix=subdir/ --add-virtual-file=good:night\n\nhere and update the test below to match.\n\n>  \ttest_when_finished \"rm -rf tmp-unpack\" &&\n>  \tmkdir tmp-unpack && (\n>  \t\tcd tmp-unpack &&\n>  \t\t\"$GIT_UNZIP\" ../with_file_with_content.zip &&\n>  \t\ttest_path_is_file hello &&\n>  \t\ttest_path_is_file \"$PATHNAME\" &&\n> -\t\ttest world = $(cat hello)\n> +\t\ttest world = $(cat hello) &&\n> +\t\ttest_path_is_file subdir/hello &&\n> +\t\ttest world = $(cat subdir/hello)\n>  \t)\n>  '\n\nOther than that, looks good to me.  Thanks.\n"},{"id":"494826","messageId":"12AA20BD-B226-43EA-A92A-BBC74FAE2840@llnl.gov","threadId":"61478","inReplyTo":"xmqqcypn14ld.fsf@gitster.g","subject":"Re: [PATCH] archive: make --add-virtual-file honor --prefix","fromName":"Tom Scogland","fromEmail":"scogland1@llnl.gov","sentAt":"2024-05-15T16:27:35Z","receivedAt":"2024-05-15T16:27:50Z","isPatch":true,"sender":{"key":"scogland1@llnl.gov","avatar":null},"body":"\n\nOn 15 May 2024, at 8:23, Junio C Hamano wrote:\n\n> \"Tom Scogland via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n>> From: Tom Scogland <scogland1@llnl.gov>\n>>\n>> The documentation for archive states:\n>>\n>>   The path of the file in the archive is built by concatenating the\n>>   value of the last `--prefix` moption (if any) before this\n>>   `--add-virtual-file` and <path>.\n>>\n>> This matches the documentation for --add-file and the behavior works for\n>> that option, but --prefix is ignored for --add-virtual-file.\n>\n> This first paragraph was a bit hard to parse for me.\n>\n> You will contrast the quoted paragraph with another one you did not\n> quote later in this paragraph, so it is more helpful to readers if\n> you instead said:\n>\n>     The documentatation for archive describes its\n>     \"--add-virtual-file\" option like so:\n>\n>         ... excerpt from --add-virtual-file description ...\n>\n>     This description is the same as \"--add-file\", and \"--add-file\"\n>     does behave the way as described.  \"--add-virtual-file\" however\n>     ignores \"--prefix\".\n\nOk, I'll update the message with this and the below style.\n\n>> This commit modifies archive.c to include the prefix in the path and\n>> adds a check into the existing add-virtual-file test to ensure that it\n>> honors both the most recent prefix before the flag.\n>\n> Style: \"This comit modifies\" -> \"Modify\".\n>\n> An obvious alternative fix is to update the documentation, which\n> would be a much safer thing to do, given that there may be existing\n> scripts written during the two years since --add-virtual-file option\n> was introduced and has been behaving exactly this way.  They will\n> all be broken big time once the command starts honoring the\n> \"--prefix\" option.\n\nI wouldn't mind doing this necessarily, but would want an option that follows the documentation in addition.  The current implementation is harder to use consistently with `--add-file` and with `--prefix`, though it's possible to manually prefix each virtual file it's surprising that it produces (as far as I have found) the only files in the archive that don't fall under the prefix.\n\n>> In looking for others with this issue, I found message\n>> a143e25a70b44b82b4ee6fa3bb2bcda4@atlas-elektronik.com on the mailing\n>> list, where Stefan proposed a basically identical patch to archive.c\n>> back in February, so the main addition here is the test along with that\n>> patch.\n>\n> This pargraph should come _after_ the three-dash lines below.\n\nCertainly.\n\n>> Signed-off-by: Tom Scogland <scogland1@llnl.gov>\n>> ---\n>>     archive: make --add-virtual-file honor --prefix\n>\n> The implementation looked obvious, assuming that it is a good idea\n> to change it (I've already talked about a safer alternative fix).\n>\n>> diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\n>> index 961c6aac256..acc8bc4fcd6 100755\n>> --- a/t/t5003-archive-zip.sh\n>> +++ b/t/t5003-archive-zip.sh\n>> @@ -218,14 +218,18 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '\n>>  \tfi &&\n>>  \tgit archive --format=zip >with_file_with_content.zip \\\n>>  \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n>> -\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n>> +\t\t--add-virtual-file=hello:world \\\n>> +\t\t--prefix=subdir/ --add-virtual-file=hello:world \\\n>> +\t\t--prefix= $EMPTY_TREE &&\n>\n> Instead of reusing the exactly the same name and contents, use\n> something different so that it is clear to the later test which of\n> the two \"--add-virtual-file\" options created these two paths in the\n> unpacked directories.  I.e., create something like\n>\n>     --prefix=subdir/ --add-virtual-file=good:night\n>\n> here and update the test below to match.\n>\n>>  \ttest_when_finished \"rm -rf tmp-unpack\" &&\n>>  \tmkdir tmp-unpack && (\n>>  \t\tcd tmp-unpack &&\n>>  \t\t\"$GIT_UNZIP\" ../with_file_with_content.zip &&\n>>  \t\ttest_path_is_file hello &&\n>>  \t\ttest_path_is_file \"$PATHNAME\" &&\n>> -\t\ttest world = $(cat hello)\n>> +\t\ttest world = $(cat hello) &&\n>> +\t\ttest_path_is_file subdir/hello &&\n>> +\t\ttest world = $(cat subdir/hello)\n>>  \t)\n>>  '\n>\n> Other than that, looks good to me.  Thanks.\n\nThanks for the feedback, I'll get an updated patch posted later today.\n"},{"id":"494991","messageId":"pull.1719.v2.git.git.1715967267420.gitgitgadget@gmail.com","threadId":"61478","inReplyTo":"pull.1719.git.git.1715721327429.gitgitgadget@gmail.com","subject":"[PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Tom Scogland via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-05-17T17:34:27Z","receivedAt":"2024-05-17T17:34:31Z","isPatch":true,"sender":{"key":"scogland1@llnl.gov","avatar":null},"body":"From: Tom Scogland <scogland1@llnl.gov>\n\nThe documentation for archive describes the `--add-virtual-file` option\nthusly:\n\n  The path of the file in the archive is built by concatenating the\n  value of the last `--prefix` moption (if any) before this\n  `--add-virtual-file` and <path>.\n\nThe `--add-file` documentation is similar:\n\n  The path of the file in the archive is built by concatenating the\n  value of the last --prefix option (if any) before this --add-file and\n  the basename of <file>.\n\nNotably both explicitly state that they honor the last `--prefix` option\nbefore the `--add` option in question.  The implementation of\n`--add-file` seems to have always honored prefix, but the implementation\nof `--add-virtual-file` does not.  Also note that `--add-virtual-file`\nexplicitly states it will use the full path given, while `--add-file`\nuses the basename of the path it is given.\n\nModify archive.c to include the prefix in the path used by\n`--add-virtual-file` and add checks into\nthe existing add-virtual-file test to verify:\n\n* that `--prefix` is honored\n* that leading path components are preserved\n* that both work together and separately\n\nChanges since v1:\n- Revised the commit message style\n- Added tests for basename/non-basename behavior\n- Fixed archive.c to use full path for virtual and basename for add-file\n\nSigned-off-by: Tom Scogland <scogland1@llnl.gov>\n---\n    archive: make --add-virtual-file honor --prefix\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1719%2Ftrws%2Fhonor-prefix-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1719/trws/honor-prefix-v2\nPull-Request: https://github.com/git/git/pull/1719\n\nRange-diff vs v1:\n\n 1:  1b685d8ca1a ! 1:  b9a0ef282a3 archive: make --add-virtual-file honor --prefix\n     @@ Metadata\n       ## Commit message ##\n          archive: make --add-virtual-file honor --prefix\n      \n     -    The documentation for archive states:\n     +    The documentation for archive describes the `--add-virtual-file` option\n     +    thusly:\n      \n            The path of the file in the archive is built by concatenating the\n            value of the last `--prefix` moption (if any) before this\n            `--add-virtual-file` and <path>.\n      \n     -    This matches the documentation for --add-file and the behavior works for\n     -    that option, but --prefix is ignored for --add-virtual-file.\n     +    The `--add-file` documentation is similar:\n      \n     -    This commit modifies archive.c to include the prefix in the path and\n     -    adds a check into the existing add-virtual-file test to ensure that it\n     -    honors both the most recent prefix before the flag.\n     +      The path of the file in the archive is built by concatenating the\n     +      value of the last --prefix option (if any) before this --add-file and\n     +      the basename of <file>.\n     +\n     +    Notably both explicitly state that they honor the last `--prefix` option\n     +    before the `--add` option in question.  The implementation of\n     +    `--add-file` seems to have always honored prefix, but the implementation\n     +    of `--add-virtual-file` does not.  Also note that `--add-virtual-file`\n     +    explicitly states it will use the full path given, while `--add-file`\n     +    uses the basename of the path it is given.\n     +\n     +    Modify archive.c to include the prefix in the path used by\n     +    `--add-virtual-file` and add checks into\n     +    the existing add-virtual-file test to verify:\n     +\n     +    * that `--prefix` is honored\n     +    * that leading path components are preserved\n     +    * that both work together and separately\n      \n     -    In looking for others with this issue, I found message\n     -    a143e25a70b44b82b4ee6fa3bb2bcda4@atlas-elektronik.com on the mailing\n     -    list, where Stefan proposed a basically identical patch to archive.c\n     -    back in February, so the main addition here is the test along with that\n     -    patch.\n     +    Changes since v1:\n     +    - Revised the commit message style\n     +    - Added tests for basename/non-basename behavior\n     +    - Fixed archive.c to use full path for virtual and basename for add-file\n      \n          Signed-off-by: Tom Scogland <scogland1@llnl.gov>\n      \n     @@ archive.c: int write_archive_entries(struct archiver_args *args,\n      +\t\tstrbuf_reset(&path_in_archive);\n      +\t\tif (info->base)\n      +\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n     -+\t\tstrbuf_addstr(&path_in_archive, basename(path));\n       \t\tif (!info->content) {\n      -\t\t\tstrbuf_reset(&path_in_archive);\n      -\t\t\tif (info->base)\n      -\t\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n     --\t\t\tstrbuf_addstr(&path_in_archive, basename(path));\n     + \t\t\tstrbuf_addstr(&path_in_archive, basename(path));\n      -\n       \t\t\tstrbuf_reset(&content);\n       \t\t\tif (strbuf_read_file(&content, path, info->stat.st_size) < 0)\n       \t\t\t\terr = error_errno(_(\"cannot read '%s'\"), path);\n      @@ archive.c: int write_archive_entries(struct archiver_args *args,\n     + \t\t\t\t\t\t  canon_mode(info->stat.st_mode),\n       \t\t\t\t\t\t  content.buf, content.len);\n       \t\t} else {\n     ++\t\t\tstrbuf_addstr(&path_in_archive, path);\n       \t\t\terr = write_entry(args, &fake_oid,\n      -\t\t\t\t\t  path, strlen(path),\n      +\t\t\t\t\t  path_in_archive.buf, path_in_archive.len,\n     @@ t/t5003-archive-zip.sh: test_expect_success UNZIP 'git archive --format=zip --ad\n       \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n      -\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n      +\t\t--add-virtual-file=hello:world \\\n     -+\t\t--prefix=subdir/ --add-virtual-file=hello:world \\\n     ++\t\t--add-virtual-file=with/dir/noprefix:withdirnopre \\\n     ++\t\t--prefix=subdir/ --add-virtual-file=with/dirprefix:withdirprefix \\\n     ++\t\t--prefix=subdir2/ --add-virtual-file=withoutdir:withoutdir \\\n      +\t\t--prefix= $EMPTY_TREE &&\n       \ttest_when_finished \"rm -rf tmp-unpack\" &&\n       \tmkdir tmp-unpack && (\n     @@ t/t5003-archive-zip.sh: test_expect_success UNZIP 'git archive --format=zip --ad\n       \t\ttest_path_is_file \"$PATHNAME\" &&\n      -\t\ttest world = $(cat hello)\n      +\t\ttest world = $(cat hello) &&\n     -+\t\ttest_path_is_file subdir/hello &&\n     -+\t\ttest world = $(cat subdir/hello)\n     ++\t\ttest_path_is_file with/dir/noprefix &&\n     ++\t\ttest withdirnopre = $(cat with/dir/noprefix) &&\n     ++\t\ttest_path_is_file subdir/with/dirprefix &&\n     ++\t\ttest withdirprefix = $(cat subdir/with/dirprefix) &&\n     ++\t\ttest_path_is_file subdir2/withoutdir &&\n     ++\t\ttest withoutdir = $(cat subdir2/withoutdir)\n       \t)\n       '\n       \n\n\n archive.c              | 10 +++++-----\n t/t5003-archive-zip.sh | 14 ++++++++++++--\n 2 files changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 5287fcdd8e0..64777a9870d 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -365,12 +365,11 @@ int write_archive_entries(struct archiver_args *args,\n \n \t\tput_be64(fake_oid.hash, i + 1);\n \n+\t\tstrbuf_reset(&path_in_archive);\n+\t\tif (info->base)\n+\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n \t\tif (!info->content) {\n-\t\t\tstrbuf_reset(&path_in_archive);\n-\t\t\tif (info->base)\n-\t\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n \t\t\tstrbuf_addstr(&path_in_archive, basename(path));\n-\n \t\t\tstrbuf_reset(&content);\n \t\t\tif (strbuf_read_file(&content, path, info->stat.st_size) < 0)\n \t\t\t\terr = error_errno(_(\"cannot read '%s'\"), path);\n@@ -380,8 +379,9 @@ int write_archive_entries(struct archiver_args *args,\n \t\t\t\t\t\t  canon_mode(info->stat.st_mode),\n \t\t\t\t\t\t  content.buf, content.len);\n \t\t} else {\n+\t\t\tstrbuf_addstr(&path_in_archive, path);\n \t\t\terr = write_entry(args, &fake_oid,\n-\t\t\t\t\t  path, strlen(path),\n+\t\t\t\t\t  path_in_archive.buf, path_in_archive.len,\n \t\t\t\t\t  canon_mode(info->stat.st_mode),\n \t\t\t\t\t  info->content, info->stat.st_size);\n \t\t}\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex 961c6aac256..0cf3aef8ace 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -218,14 +218,24 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '\n \tfi &&\n \tgit archive --format=zip >with_file_with_content.zip \\\n \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n-\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n+\t\t--add-virtual-file=hello:world \\\n+\t\t--add-virtual-file=with/dir/noprefix:withdirnopre \\\n+\t\t--prefix=subdir/ --add-virtual-file=with/dirprefix:withdirprefix \\\n+\t\t--prefix=subdir2/ --add-virtual-file=withoutdir:withoutdir \\\n+\t\t--prefix= $EMPTY_TREE &&\n \ttest_when_finished \"rm -rf tmp-unpack\" &&\n \tmkdir tmp-unpack && (\n \t\tcd tmp-unpack &&\n \t\t\"$GIT_UNZIP\" ../with_file_with_content.zip &&\n \t\ttest_path_is_file hello &&\n \t\ttest_path_is_file \"$PATHNAME\" &&\n-\t\ttest world = $(cat hello)\n+\t\ttest world = $(cat hello) &&\n+\t\ttest_path_is_file with/dir/noprefix &&\n+\t\ttest withdirnopre = $(cat with/dir/noprefix) &&\n+\t\ttest_path_is_file subdir/with/dirprefix &&\n+\t\ttest withdirprefix = $(cat subdir/with/dirprefix) &&\n+\t\ttest_path_is_file subdir2/withoutdir &&\n+\t\ttest withoutdir = $(cat subdir2/withoutdir)\n \t)\n '\n \n\nbase-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8\n-- \ngitgitgadget\n"},{"id":"495011","messageId":"xmqqv83cdneo.fsf@gitster.g","threadId":"61478","inReplyTo":"pull.1719.v2.git.git.1715967267420.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-17T23:33:19Z","receivedAt":"2024-05-17T23:33:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tom Scogland via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Notably both explicitly state that they honor the last `--prefix` option\n> before the `--add` option in question.  The implementation of\n> `--add-file` seems to have always honored prefix, but the implementation\n> of `--add-virtual-file` does not.\n\nThe above is misleading.\n\n    The implementation of `--add-file` has always honored the prefix,\n    while the implementation of `--add-virtual-file` has always ignored\n    the prefix.\n\nwould make it easier to assess how long existing users may have been\nrelying on the current behaviour.\n\n> Also note that `--add-virtual-file`\n> explicitly states it will use the full path given, while `--add-file`\n> uses the basename of the path it is given.\n\nYes, this is a very good thing to mention.  It is probably the\nreason why the implementation decided not to add prefix to the \"full\npath\" that already can have the leading directories.\n\n> Modify archive.c to include the prefix in the path used by\n> `--add-virtual-file` and add checks into\n> the existing add-virtual-file test to verify:\n>\n> * that `--prefix` is honored\n> * that leading path components are preserved\n> * that both work together and separately\n\nVery nice job explaining the chosen design clearly (even though I do\nnot necessarily agree with the direction this patch is going).\n\nAlso, given that this option was introduced for an explicit purpose\nof using it to write out diagnostics archive file, we should mention\nthat this change does not break it in the proposed log message, at\nleast.  Of course, we should do so after verifying that is indeed\nthe case, and better yet, after verifying that it will be hard for\nfuture changes to diagnose.c to trigger an unexpected behaviour\ncaused by this change [*].\n\n> Changes since v1:\n> - Revised the commit message style\n> - Added tests for basename/non-basename behavior\n> - Fixed archive.c to use full path for virtual and basename for add-file\n\nThe \"changes since v1\" section does not belong to the log message\nproper, as v1 never happened as long as readers of \"git log\" are\nconcerned.  It is a very good thing to help reviewers to have below\nthe three-dash lines that comes after your sign-off, though.\n\n> Signed-off-by: Tom Scogland <scogland1@llnl.gov>\n> ---\n\n>  archive.c              | 10 +++++-----\n>  t/t5003-archive-zip.sh | 14 ++++++++++++--\n>  2 files changed, 17 insertions(+), 7 deletions(-)\n>\n> diff --git a/archive.c b/archive.c\n> index 5287fcdd8e0..64777a9870d 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -365,12 +365,11 @@ int write_archive_entries(struct archiver_args *args,\n>  \n>  \t\tput_be64(fake_oid.hash, i + 1);\n>  \n> +\t\tstrbuf_reset(&path_in_archive);\n> +\t\tif (info->base)\n> +\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n>  \t\tif (!info->content) {\n> -\t\t\tstrbuf_reset(&path_in_archive);\n> -\t\t\tif (info->base)\n> -\t\t\t\tstrbuf_addstr(&path_in_archive, info->base);\n>  \t\t\tstrbuf_addstr(&path_in_archive, basename(path));\n> -\n>  \t\t\tstrbuf_reset(&content);\n>  \t\t\tif (strbuf_read_file(&content, path, info->stat.st_size) < 0)\n>  \t\t\t\terr = error_errno(_(\"cannot read '%s'\"), path);\n> @@ -380,8 +379,9 @@ int write_archive_entries(struct archiver_args *args,\n>  \t\t\t\t\t\t  canon_mode(info->stat.st_mode),\n>  \t\t\t\t\t\t  content.buf, content.len);\n>  \t\t} else {\n> +\t\t\tstrbuf_addstr(&path_in_archive, path);\n>  \t\t\terr = write_entry(args, &fake_oid,\n> -\t\t\t\t\t  path, strlen(path),\n> +\t\t\t\t\t  path_in_archive.buf, path_in_archive.len,\n>  \t\t\t\t\t  canon_mode(info->stat.st_mode),\n>  \t\t\t\t\t  info->content, info->stat.st_size);\n>  \t\t}\n> diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\n> index 961c6aac256..0cf3aef8ace 100755\n> --- a/t/t5003-archive-zip.sh\n> +++ b/t/t5003-archive-zip.sh\n> @@ -218,14 +218,24 @@ test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '\n>  \tfi &&\n>  \tgit archive --format=zip >with_file_with_content.zip \\\n>  \t\t--add-virtual-file=\\\"\"$PATHNAME\"\\\": \\\n> -\t\t--add-virtual-file=hello:world $EMPTY_TREE &&\n> +\t\t--add-virtual-file=hello:world \\\n> +\t\t--add-virtual-file=with/dir/noprefix:withdirnopre \\\n> +\t\t--prefix=subdir/ --add-virtual-file=with/dirprefix:withdirprefix \\\n> +\t\t--prefix=subdir2/ --add-virtual-file=withoutdir:withoutdir \\\n> +\t\t--prefix= $EMPTY_TREE &&\n>  \ttest_when_finished \"rm -rf tmp-unpack\" &&\n>  \tmkdir tmp-unpack && (\n>  \t\tcd tmp-unpack &&\n>  \t\t\"$GIT_UNZIP\" ../with_file_with_content.zip &&\n>  \t\ttest_path_is_file hello &&\n>  \t\ttest_path_is_file \"$PATHNAME\" &&\n> -\t\ttest world = $(cat hello)\n> +\t\ttest world = $(cat hello) &&\n> +\t\ttest_path_is_file with/dir/noprefix &&\n> +\t\ttest withdirnopre = $(cat with/dir/noprefix) &&\n> +\t\ttest_path_is_file subdir/with/dirprefix &&\n> +\t\ttest withdirprefix = $(cat subdir/with/dirprefix) &&\n> +\t\ttest_path_is_file subdir2/withoutdir &&\n> +\t\ttest withoutdir = $(cat subdir2/withoutdir)\n\nOK.  With different payload at different paths, it is easier than\nthe previous round to see where things are expected to go in the\nresult.\n\n\n[Footnote]\n\n * I got curious and did this part for you.  After calling\n   add_directory_to_archiver() that uses \"--prefix\" to move the\n   target directory around in the output hierarchy, the code clears\n   with \"--prefix=\"---even a future change to diagnose.c adds more\n   uses to --add-virtual-file= after it happens, it will not go to\n   deep in the directory hierarchy where the last use of \"--prefix\"\n   happened to be pointing at.\n\n"},{"id":"495020","messageId":"176A8590-61BC-45F2-9B37-FFD8B07ABCA9@llnl.gov","threadId":"61478","inReplyTo":"xmqqv83cdneo.fsf@gitster.g","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Tom Scogland","fromEmail":"scogland1@llnl.gov","sentAt":"2024-05-18T00:26:08Z","receivedAt":"2024-05-18T00:26:23Z","isPatch":true,"sender":{"key":"scogland1@llnl.gov","avatar":null},"body":"\n\nOn 17 May 2024, at 16:33, Junio C Hamano wrote:\n\n> \"Tom Scogland via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n>> Notably both explicitly state that they honor the last `--prefix` option\n>> before the `--add` option in question.  The implementation of\n>> `--add-file` seems to have always honored prefix, but the implementation\n>> of `--add-virtual-file` does not.\n>\n> The above is misleading.\n>\n>     The implementation of `--add-file` has always honored the prefix,\n>     while the implementation of `--add-virtual-file` has always ignored\n>     the prefix.\n>\n> would make it easier to assess how long existing users may have been\n> relying on the current behaviour.\n\nFair, I had no intention to mislead and will reword.\n\n>> Modify archive.c to include the prefix in the path used by\n>> `--add-virtual-file` and add checks into\n>> the existing add-virtual-file test to verify:\n>>\n>> * that `--prefix` is honored\n>> * that leading path components are preserved\n>> * that both work together and separately\n>\n> Very nice job explaining the chosen design clearly (even though I do\n> not necessarily agree with the direction this patch is going).\n\nThanks for that.  As to the direction, I mentioned earlier adding a different flag, or perhaps marking the filename in some fashion to express that the prefix should be honored, would you prefer that? It would, as you said, be much safer in that there's no reason for it to be a breaking change. If there's a design you prefer that would result in having an opt-in way to get the prefix behavior I wouldn't mind implementing it.\n\n> Also, given that this option was introduced for an explicit purpose\n> of using it to write out diagnostics archive file, we should mention\n> that this change does not break it in the proposed log message, at\n> least.  Of course, we should do so after verifying that is indeed\n> the case, and better yet, after verifying that it will be hard for\n> future changes to diagnose.c to trigger an unexpected behaviour\n> caused by this change [*].\n\nThat's a very good point, and thank you for digging into it.\n\n>> Changes since v1:\n>> - Revised the commit message style\n>> - Added tests for basename/non-basename behavior\n>> - Fixed archive.c to use full path for virtual and basename for add-file\n>\n> The \"changes since v1\" section does not belong to the log message\n> proper, as v1 never happened as long as readers of \"git log\" are\n> concerned.  It is a very good thing to help reviewers to have below\n> the three-dash lines that comes after your sign-off, though.\n\nMy apologies, this is my unfamiliarity with GitGitGadget, I'll put information like this in the PR description next time, which I think will do that.\n"},{"id":"495060","messageId":"bc3711a7-37d5-46bc-979e-83bd0b2cf900@web.de","threadId":"61478","inReplyTo":"pull.1719.v2.git.git.1715967267420.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-05-19T13:25:27Z","receivedAt":"2024-05-19T13:25:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 17.05.24 um 19:34 schrieb Tom Scogland via GitGitGadget:\n> From: Tom Scogland <scogland1@llnl.gov>\n>\n> The documentation for archive describes the `--add-virtual-file` option\n> thusly:\n>\n>   The path of the file in the archive is built by concatenating the\n>   value of the last `--prefix` moption (if any) before this\n>   `--add-virtual-file` and <path>.\n\nThe documentation does not actually misspell \"option\" as \"moption\".\n\n> The `--add-file` documentation is similar:\n>\n>   The path of the file in the archive is built by concatenating the\n>   value of the last --prefix option (if any) before this --add-file and\n>   the basename of <file>.\n>\n> Notably both explicitly state that they honor the last `--prefix` option\n> before the `--add` option in question.  The implementation of\n> `--add-file` seems to have always honored prefix, but the implementation\n> of `--add-virtual-file` does not.  Also note that `--add-virtual-file`\n> explicitly states it will use the full path given, while `--add-file`\n> uses the basename of the path it is given.\n>\n> Modify archive.c to include the prefix in the path used by\n> `--add-virtual-file`\n\nAligning code and docs is a good idea.  Have you considered keeping the\ncode as is and changing the documentation instead, though?\n\nThe two options are related in that they both add untracked files, but\nthey necessarily have different arguments:\n\n   --add-file=<file>\n   --add-virtual-file=<path>:<content>\n\nYou can already specify any path you want with --add-virtual-file.\nWhat's the advantage of honoring --prefix as well?\n\nRené\n\n"},{"id":"495085","messageId":"95B94723-BCBF-48B4-A02C-A5C61FB060DA@llnl.gov","threadId":"61478","inReplyTo":"bc3711a7-37d5-46bc-979e-83bd0b2cf900@web.de","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Tom Scogland","fromEmail":"scogland1@llnl.gov","sentAt":"2024-05-20T16:10:51Z","receivedAt":"2024-05-20T16:11:01Z","isPatch":true,"sender":{"key":"scogland1@llnl.gov","avatar":null},"body":"\n\nOn 19 May 2024, at 6:25, René Scharfe wrote:\n\n>\n> Aligning code and docs is a good idea.  Have you considered keeping the\n> code as is and changing the documentation instead, though?\n>\n> The two options are related in that they both add untracked files, but\n> they necessarily have different arguments:\n>\n>    --add-file=<file>\n>    --add-virtual-file=<path>:<content>\n>\n> You can already specify any path you want with --add-virtual-file.\n> What's the advantage of honoring --prefix as well?\n\nI came into this after trying to translate an --add-file to an --add-virtual-file and being surprised that the prefix wasn't applied. anything else you add, in the repo or not, it gets the prefix. I can go back and explicitly add the prefix, but it makes the options less naturally composable in my opinion.  The specific case was packaging some software that uses git metadata to set version and some other things when in a repo, and files in a tarball. The tarball's prefix is set in one part of the packaging code, files and exclusions in another, and passing that through was something we didn't have to think about using add-file, but do with add-virtual-file.\n\nThat said, it sounds like both you and Junio prefer updating the docs rather than the code, which makes me think I'm in the minority in that opinion.  If that's the case, I can certainly update the docs, and I imagine we can backport that easily wherever it makes sense.  I would really like to have the option to have the prefix apply though, either adding a new flag or an option to the existing one that would be invalid given current syntax or similar to provide the option.\n"},{"id":"495091","messageId":"01b21870-b37e-47fe-9bd7-225499ae5eaf@web.de","threadId":"61478","inReplyTo":"95B94723-BCBF-48B4-A02C-A5C61FB060DA@llnl.gov","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-05-20T17:07:22Z","receivedAt":"2024-05-20T17:07:38Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.05.24 um 18:10 schrieb Tom Scogland:\n>\n> On 19 May 2024, at 6:25, René Scharfe wrote:\n>\n>> You can already specify any path you want with --add-virtual-file.\n>> What's the advantage of honoring --prefix as well?\n>\n> I came into this after trying to translate an --add-file to an\n> --add-virtual-file and being surprised that the prefix wasn't\n> applied.\n\nUnderstandable, the documentation promised otherwise and the options\nhave very similar names.\n\n> anything else you add, in the repo or not, it gets the prefix.> I can go back and explicitly add the prefix, but it makes the\n> options less naturally composable in my opinion.\n\nTrue, applying the prefix to all items would be simpler overall.\n\nSpeaking of simpler: --add-virtual-file could have been implemented to\nonly take a single argument -- the content -- and rely on --prefix to\nprovide the full path.  That's more consistent with other options, as\nmost of them only take single-valued arguments (or none). :]\n\n> That said, it sounds like both you and Junio prefer updating the docs\n> rather than the code, which makes me think I'm in the minority in\n> that opinion.\n\nI'm not sure I have an opinion on that topic, yet.  Fixing the\ndocumentation is certainly easier.  Adding the prefix to the path of\nvirtual files as well is a breaking change.  I feel that the easier\nroute should at least be mentioned in the commit message and why it\nwas not taken.\n\n> If that's the case, I can certainly update the docs, and I imagine we\n> can backport that easily wherever it makes sense.  I would really\n> like to have the option to have the prefix apply though, either\n> adding a new flag or an option to the existing one that would be\n> invalid given current syntax or similar to provide the option.\nYou mean like replacing a leading colon in the path with the prefix?\n\nRené\n"},{"id":"495094","messageId":"xmqq5xv8id0x.fsf@gitster.g","threadId":"61478","inReplyTo":"95B94723-BCBF-48B4-A02C-A5C61FB060DA@llnl.gov","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-20T17:55:26Z","receivedAt":"2024-05-20T17:55:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Scogland <scogland1@llnl.gov> writes:\n\n> That said, it sounds like both you and Junio prefer updating the\n> docs rather than the code, which makes me think I'm in the\n> minority in that opinion.  If that's the case, I can certainly\n> update the docs, and I imagine we can backport that easily\n> wherever it makes sense.  I would really like to have the option\n> to have the prefix apply though, either adding a new flag or an\n> option to the existing one that would be invalid given current\n> syntax or similar to provide the option.\n\n[jc: wrapped overly long lines]\n\nWouldn't\n\n    #!/bin/sh\n    prefix=foo/bar\n    git archive --prefix=\"$prefix\" --add-file=x --add-file=y \\\n\t--add-virtual-file=\"$prefix/path:contents\"\n\nbe an option enough?  You only have to define $prefix once.\n\n"},{"id":"497183","messageId":"xmqqh6dvflea.fsf@gitster.g","threadId":"61478","inReplyTo":"01b21870-b37e-47fe-9bd7-225499ae5eaf@web.de","subject":"Re: [PATCH v2] archive: make --add-virtual-file honor --prefix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-14T18:07:41Z","receivedAt":"2024-06-14T18:07:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> I'm not sure I have an opinion on that topic, yet.  Fixing the\n> documentation is certainly easier.  Adding the prefix to the path of\n> virtual files as well is a breaking change.  I feel that the easier\n> route should at least be mentioned in the commit message and why it\n> was not taken.\n\nIt has been a few weeks since this discussion stalled.  Let me make\nan executive decision on the direction here---let's keep the behaviour\nand align the documentation so that we won't break existing users.\n\nThanks.\n"},{"id":"497185","messageId":"xmqq5xubfjuu.fsf_-_@gitster.g","threadId":"61478","inReplyTo":"xmqqh6dvflea.fsf@gitster.g","subject":"[PATCH] archive: document that --add-virtual-file takes full path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-14T18:40:57Z","receivedAt":"2024-06-14T18:41:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> René Scharfe <l.s.r@web.de> writes:\n>\n>> I'm not sure I have an opinion on that topic, yet.  Fixing the\n>> documentation is certainly easier.  Adding the prefix to the path of\n>> virtual files as well is a breaking change.  I feel that the easier\n>> route should at least be mentioned in the commit message and why it\n>> was not taken.\n>\n> It has been a few weeks since this discussion stalled.  Let me make\n> an executive decision on the direction here---let's keep the behaviour\n> and align the documentation so that we won't break existing users.\n>\n> Thanks.\n\nSo here is to re-ignite the discussion.\n\n----- >8 -----\nSubject: [PATCH] archive: document that --add-virtual-file takes full path\n\nTom Scogland noticed that `--add-virtual-file` option uses the path\nspecified as its value as-is, without prepending any value given to\nthe `--prefix` option like `--add-file` does.\n\nThe behaviour has always been that way since the option was\nintroduced, but the documentation has always been wrong and said\nthat it would use the value of `--prefix` just like `--add-file`\ndoes.\n\nWe could modify the behaviour to make it literally work like the\ndocumentation said, but it would break existing scripts the users\nuse.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-archive.txt | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 98526f2beb..a0e3fe7996 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -53,7 +53,7 @@ OPTIONS\n --prefix=<prefix>/::\n \tPrepend <prefix>/ to paths in the archive.  Can be repeated; its\n \trightmost value is used for all tracked files.  See below which\n-\tvalue gets used by `--add-file` and `--add-virtual-file`.\n+\tvalue gets used by `--add-file`.\n \n -o <file>::\n --output=<file>::\n@@ -67,9 +67,7 @@ OPTIONS\n \n --add-virtual-file=<path>:<content>::\n \tAdd the specified contents to the archive.  Can be repeated to add\n-\tmultiple files.  The path of the file in the archive is built\n-\tby concatenating the value of the last `--prefix` option (if any)\n-\tbefore this `--add-virtual-file` and `<path>`.\n+\tmultiple files.\n +\n The `<path>` argument can start and end with a literal double-quote\n character; the contained file name is interpreted as a C-style string,\n@@ -81,6 +79,10 @@ if the path begins or ends with a double-quote character.\n The file mode is limited to a regular file, and the option may be\n subject to platform-dependent command-line limits. For non-trivial\n cases, write an untracked file and use `--add-file` instead.\n++\n+Note that unlike `--add-file` the path created in the archive is not\n+affected by the `--prefix` option, as a full `<path>` can be given as\n+the value of the option.\n \n --worktree-attributes::\n \tLook for attributes in .gitattributes files in the working tree\n-- \n2.45.2-683-g09b5b61c39\n\n"},{"id":"497660","messageId":"xmqq8qys7iex.fsf@gitster.g","threadId":"61478","inReplyTo":"xmqq5xubfjuu.fsf_-_@gitster.g","subject":"Re: [PATCH] archive: document that --add-virtual-file takes full path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-25T22:43:18Z","receivedAt":"2024-06-25T22:43:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> It has been a few weeks since this discussion stalled.  Let me make\n>> an executive decision on the direction here---let's keep the behaviour\n>> and align the documentation so that we won't break existing users.\n>>\n>> Thanks.\n>\n> So here is to re-ignite the discussion.\n>\n> ----- >8 -----\n> Subject: [PATCH] archive: document that --add-virtual-file takes full path\n\nAnd it seems that nobody is interested in the topic all that much\nanymore?\n\nI've read the updated text once again, and didn't see anything\nglaringly wrong, so I'll mark it for 'next'.\n\nThanks.\n"},{"id":"497722","messageId":"d1842d42-6104-49f0-99b6-fc9095a61b6a@web.de","threadId":"61478","inReplyTo":"xmqq8qys7iex.fsf@gitster.g","subject":"Re: [PATCH] archive: document that --add-virtual-file takes full path","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-06-26T19:05:54Z","receivedAt":"2024-06-26T19:06:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.06.24 um 00:43 schrieb Junio C Hamano:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> It has been a few weeks since this discussion stalled.  Let me make\n>>> an executive decision on the direction here---let's keep the behaviour\n>>> and align the documentation so that we won't break existing users.\n>>>\n>>> Thanks.\n>>\n>> So here is to re-ignite the discussion.\n>>\n>> ----- >8 -----\n>> Subject: [PATCH] archive: document that --add-virtual-file takes full path\n>\n> And it seems that nobody is interested in the topic all that much\n> anymore?\n\nI'm low-key pleased to see a documentation bug being stomped.\n\nPerhaps this saps the energy out of efforts to make --add-virtual-file\nrespect --prefix, but we'd probably need a new option name for that\nanyway to keep backward compatibility (--add-prefix-file?), so the patch\nadds no technical obstacle for that at least.\n\n> I've read the updated text once again, and didn't see anything\n> glaringly wrong, so I'll mark it for 'next'.\n\nThe patch looks good to me.  I like the new paragraph about --prefix not\nbeing respected, in particular because I wouldn't thought of adding it,\nbut it does look helpful.\n\nPerhaps add a Reported-by: line for Tom that's findable with trailer\ntooling, though?\n\nRené\n"}]}