From: "René Scharfe" <l.s.r@web.de> To: Junio C Hamano <gitster@pobox.com>, Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>, git@vger.kernel.org, Taylor Blau <me@ttaylorr.com>, Derrick Stolee <stolee@gmail.com>, Elijah Newren <newren@gmail.com> Subject: Re: [PATCH v2 1/6] archive: optionally add "virtual" files Date: Tue, 8 Feb 2022 21:58:16 +0100 [thread overview] Message-ID: <b49d396d-a433-51a4-2d19-55e175af571a@web.de> (raw) In-Reply-To: <xmqqbkzhdzib.fsf@gitster.g> Am 08.02.22 um 18:44 schrieb Junio C Hamano: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >>>> We could use that option in Git's own Makefile to add the file named >>>> "version", which contains $GIT_VERSION. Hmm, but it also contains a >>>> terminating newline, which would be a bit tricky (but not impossible) to >>>> add. Would it make sense to add one automatically if it's missing (e.g. >>>> with strbuf_complete_line)? Not sure. >>> >>> I do not think it is a good UI to give raw file content from the >>> command line, which will be usable only for trivial, even single >>> liner files, and forces people to learn two parallel option, one >>> for trivial ones and the other for contents with meaningful size. >> >> Nevertheless, it is still the most elegant way that I can think of to >> generate a diagnostic `.zip` file without messing up the very things that >> are to be diagnosed: the repository and the worktree. > > Puzzled. Are you feeding contents of a .zip file from the command > line? Kind of. Command line arguments are built and handed to write_archive() in-process. It's done by patch 3 and extended by 5 and 6. The number of files is relatively low and they aren't huge, right? Staging their content in the object database would be messy, but $TMPDIR might be able to take them with a low impact. Unless the problem to diagnose is that this directory is full -- but you don't need a fancy report for that. :) Currently there is no easy way to write a temporary file with a chosen name. diff.c would benefit from such a thing when running an external diff program; currently it adds a random prefix. git archive --add-file also uses the filename (and discards the directory part). The patch below adds a function to create temporary files with a chosen name. Perhaps it would be useful here as well, instead of the new option? > I was mostly worried about busting command line argument limit by > trying to feed too many bytes, as the ceiling is fairly low on some > platforms. Command line length limits don't apply to the way scalar uses the new option. > Another worry was that when <contents> can have > arbitrary bytes, with --opt=<path>:<contents> syntax, the input > becomes ambiguous (i.e. "which colon is the <path> separator?"), > without some way to escape a colon in the payload. The first colon is the separator here. > For a single-liner, --add-file-with-contents=<path>:<contents> would > be an OK way, and my comment was not a strong objection against this > new option existing. It was primarily an objection against changing > the way to add the 'version' file in our "make dist" procedure to > use it anyway. > > But now I think about it more, I am becoming less happy about it > existing in the first place. > > This will throw another monkey wrench to Konstantin's plan [*] to > make "git archive" output verifiable with the signature on original > Git objects, but it is not a new problem ;-) > > > [Reference] > > * https://lore.kernel.org/git/20220207213449.ljqjhdx4f45a3lx5@meerkat.local/ I don't see the conflict: If an untracked file is added to an archive using --add-file, --add-file-with-content, or ZIP or tar then we'd *want* the verification against a signed commit or tag to fail, no? A different signature would be required for the non-tracked parts. René --- >8 --- Subject: [PATCH] tempfile: add mks_tempfile_dt() Add a function to create a temporary file with a certain name in a temporary directory created using mkdtemp(3). Its result is more sightly than the paths created by mks_tempfile_ts(), which include a random prefix. That's useful for files passed to a program that displays their name, e.g. an external diff tool. Signed-off-by: René Scharfe <l.s.r@web.de> --- tempfile.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tempfile.h | 13 +++++++++++ 2 files changed, 76 insertions(+) diff --git a/tempfile.c b/tempfile.c index 94aa18f3f7..2024c82691 100644 --- a/tempfile.c +++ b/tempfile.c @@ -56,6 +56,20 @@ static VOLATILE_LIST_HEAD(tempfile_list); +static void remove_template_directory(struct tempfile *tempfile, + int in_signal_handler) +{ + if (tempfile->directorylen > 0 && + tempfile->directorylen < tempfile->filename.len && + tempfile->filename.buf[tempfile->directorylen] == '/') { + strbuf_setlen(&tempfile->filename, tempfile->directorylen); + if (in_signal_handler) + rmdir(tempfile->filename.buf); + else + rmdir_or_warn(tempfile->filename.buf); + } +} + static void remove_tempfiles(int in_signal_handler) { pid_t me = getpid(); @@ -74,6 +88,7 @@ static void remove_tempfiles(int in_signal_handler) unlink(p->filename.buf); else unlink_or_warn(p->filename.buf); + remove_template_directory(p, in_signal_handler); p->active = 0; } @@ -100,6 +115,7 @@ static struct tempfile *new_tempfile(void) tempfile->owner = 0; INIT_LIST_HEAD(&tempfile->list); strbuf_init(&tempfile->filename, 0); + tempfile->directorylen = 0; return tempfile; } @@ -198,6 +214,52 @@ struct tempfile *mks_tempfile_tsm(const char *filename_template, int suffixlen, return tempfile; } +struct tempfile *mks_tempfile_dt(const char *directory_template, + const char *filename) +{ + struct tempfile *tempfile; + const char *tmpdir; + struct strbuf sb = STRBUF_INIT; + int fd; + size_t directorylen; + + if (!ends_with(directory_template, "XXXXXX")) { + errno = EINVAL; + return NULL; + } + + tmpdir = getenv("TMPDIR"); + if (!tmpdir) + tmpdir = "/tmp"; + + strbuf_addf(&sb, "%s/%s", tmpdir, directory_template); + directorylen = sb.len; + if (!mkdtemp(sb.buf)) { + int orig_errno = errno; + strbuf_release(&sb); + errno = orig_errno; + return NULL; + } + + strbuf_addf(&sb, "/%s", filename); + fd = open(sb.buf, O_CREAT | O_EXCL | O_RDWR, 0600); + if (fd < 0) { + int orig_errno = errno; + strbuf_setlen(&sb, directorylen); + rmdir(sb.buf); + strbuf_release(&sb); + errno = orig_errno; + return NULL; + } + + tempfile = new_tempfile(); + strbuf_swap(&tempfile->filename, &sb); + tempfile->directorylen = directorylen; + tempfile->fd = fd; + activate_tempfile(tempfile); + return tempfile; +} + struct tempfile *xmks_tempfile_m(const char *filename_template, int mode) { struct tempfile *tempfile; @@ -316,6 +378,7 @@ void delete_tempfile(struct tempfile **tempfile_p) close_tempfile_gently(tempfile); unlink_or_warn(tempfile->filename.buf); + remove_template_directory(tempfile, 0); deactivate_tempfile(tempfile); *tempfile_p = NULL; } diff --git a/tempfile.h b/tempfile.h index 4de3bc77d2..d7804a214a 100644 --- a/tempfile.h +++ b/tempfile.h @@ -82,6 +82,7 @@ struct tempfile { FILE *volatile fp; volatile pid_t owner; struct strbuf filename; + size_t directorylen; }; /* @@ -198,6 +199,18 @@ static inline struct tempfile *xmks_tempfile(const char *filename_template) return xmks_tempfile_m(filename_template, 0600); } +/* + * Attempt to create a temporary directory in $TMPDIR and to create and + * open a file in that new directory. Derive the directory name from the + * template in the manner of mkdtemp(). Arrange for directory and file + * to be deleted if the program exits before they are deleted + * explicitly. On success return a tempfile whose "filename" member + * contains the full path of the file and its "fd" member is open for + * writing the file. On error return NULL and set errno appropriately. + */ +struct tempfile *mks_tempfile_dt(const char *directory_template, + const char *filename); + /* * Associate a stdio stream with the temporary file (which must still * be open). Return `NULL` (*without* deleting the file) on error. The -- 2.35.1
next prev parent reply other threads:[~2022-02-08 22:25 UTC|newest] Thread overview: 140+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-26 8:41 [PATCH 0/5] scalar: implement the subcommand "diagnose" Johannes Schindelin via GitGitGadget 2022-01-26 8:41 ` [PATCH 1/5] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-01-26 9:34 ` René Scharfe 2022-01-26 22:20 ` Taylor Blau 2022-02-06 21:34 ` Johannes Schindelin 2022-01-27 19:38 ` Elijah Newren 2022-01-26 8:41 ` [PATCH 2/5] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-01-26 8:41 ` [PATCH 3/5] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-01-26 22:43 ` Taylor Blau 2022-01-27 15:14 ` Derrick Stolee 2022-02-06 21:38 ` Johannes Schindelin 2022-01-26 8:41 ` [PATCH 4/5] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-01-26 22:50 ` Taylor Blau 2022-01-27 15:17 ` Derrick Stolee 2022-01-27 18:59 ` Elijah Newren 2022-02-06 21:25 ` Johannes Schindelin 2022-01-26 8:41 ` [PATCH 5/5] scalar diagnose: show a spinner while staging content Johannes Schindelin via GitGitGadget 2022-01-27 15:19 ` [PATCH 0/5] scalar: implement the subcommand "diagnose" Derrick Stolee 2022-02-06 21:13 ` Johannes Schindelin 2022-02-06 22:39 ` [PATCH v2 0/6] " Johannes Schindelin via GitGitGadget 2022-02-06 22:39 ` [PATCH v2 1/6] archive: optionally add "virtual" files Johannes Schindelin via GitGitGadget 2022-02-07 19:55 ` René Scharfe 2022-02-07 23:30 ` Junio C Hamano 2022-02-08 13:12 ` Johannes Schindelin 2022-02-08 17:44 ` Junio C Hamano 2022-02-08 20:58 ` René Scharfe [this message] 2022-02-09 22:48 ` Junio C Hamano 2022-02-10 19:10 ` René Scharfe 2022-02-10 19:23 ` Junio C Hamano 2022-02-11 19:16 ` René Scharfe 2022-02-11 21:27 ` Junio C Hamano 2022-02-12 9:12 ` René Scharfe 2022-02-13 6:25 ` Junio C Hamano 2022-02-13 9:02 ` René Scharfe 2022-02-14 17:22 ` Junio C Hamano 2022-02-08 12:54 ` Johannes Schindelin 2022-02-06 22:39 ` [PATCH v2 2/6] scalar: validate the optional enlistment argument Johannes Schindelin via GitGitGadget 2022-02-06 22:39 ` [PATCH v2 3/6] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-02-07 19:55 ` René Scharfe 2022-02-08 12:08 ` Johannes Schindelin 2022-02-06 22:39 ` [PATCH v2 4/6] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-02-06 22:39 ` [PATCH v2 5/6] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-02-06 22:39 ` [PATCH v2 6/6] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 0/7] scalar: implement the subcommand "diagnose" Johannes Schindelin via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 1/7] archive: optionally add "virtual" files Johannes Schindelin via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 2/7] archive --add-file-with-contents: allow paths containing colons Johannes Schindelin via GitGitGadget 2022-05-07 2:06 ` Elijah Newren 2022-05-09 21:04 ` Johannes Schindelin 2022-05-04 15:25 ` [PATCH v3 3/7] scalar: validate the optional enlistment argument Johannes Schindelin via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 4/7] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 5/7] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 6/7] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-05-04 15:25 ` [PATCH v3 7/7] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-05-07 2:23 ` [PATCH v3 0/7] scalar: implement the subcommand "diagnose" Elijah Newren 2022-05-10 19:26 ` [PATCH v4 " Johannes Schindelin via GitGitGadget 2022-05-10 19:26 ` [PATCH v4 1/7] archive: optionally add "virtual" files Johannes Schindelin via GitGitGadget 2022-05-10 21:48 ` Junio C Hamano 2022-05-10 22:06 ` rsbecker 2022-05-10 23:21 ` Junio C Hamano 2022-05-11 16:14 ` René Scharfe 2022-05-11 19:27 ` Junio C Hamano 2022-05-12 16:16 ` René Scharfe 2022-05-12 18:15 ` Junio C Hamano 2022-05-12 21:31 ` Junio C Hamano 2022-05-14 7:06 ` René Scharfe 2022-05-12 22:31 ` [PATCH] fixup! " Junio C Hamano 2022-05-10 19:26 ` [PATCH v4 2/7] archive --add-file-with-contents: allow paths containing colons Johannes Schindelin via GitGitGadget 2022-05-10 21:56 ` Junio C Hamano 2022-05-10 22:23 ` rsbecker 2022-05-19 18:12 ` Johannes Schindelin 2022-05-19 18:09 ` Johannes Schindelin 2022-05-19 18:44 ` Junio C Hamano 2022-05-10 19:27 ` [PATCH v4 3/7] scalar: validate the optional enlistment argument Johannes Schindelin via GitGitGadget 2022-05-17 14:51 ` Ævar Arnfjörð Bjarmason 2022-05-18 17:35 ` Junio C Hamano 2022-05-20 7:30 ` Ævar Arnfjörð Bjarmason 2022-05-20 15:55 ` Johannes Schindelin 2022-05-21 9:54 ` Ævar Arnfjörð Bjarmason 2022-05-22 5:50 ` Junio C Hamano 2022-05-24 12:25 ` Johannes Schindelin 2022-05-24 18:11 ` Ævar Arnfjörð Bjarmason 2022-05-24 19:29 ` Junio C Hamano 2022-05-25 10:31 ` Johannes Schindelin 2022-05-10 19:27 ` [PATCH v4 4/7] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-05-17 14:53 ` Ævar Arnfjörð Bjarmason 2022-05-10 19:27 ` [PATCH v4 5/7] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-05-10 19:27 ` [PATCH v4 6/7] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-05-10 19:27 ` [PATCH v4 7/7] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-05-17 15:03 ` [PATCH v4 0/7] scalar: implement the subcommand "diagnose" Ævar Arnfjörð Bjarmason 2022-05-17 15:28 ` rsbecker 2022-05-19 18:17 ` Johannes Schindelin 2022-05-19 18:17 ` [PATCH v5 " Johannes Schindelin via GitGitGadget 2022-05-19 18:17 ` [PATCH v5 1/7] archive: optionally add "virtual" files Johannes Schindelin via GitGitGadget 2022-05-20 14:41 ` René Scharfe 2022-05-20 16:21 ` Junio C Hamano 2022-05-19 18:17 ` [PATCH v5 2/7] archive --add-file-with-contents: allow paths containing colons Johannes Schindelin via GitGitGadget 2022-05-19 18:17 ` [PATCH v5 3/7] scalar: validate the optional enlistment argument Johannes Schindelin via GitGitGadget 2022-05-19 18:18 ` [PATCH v5 4/7] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-05-19 18:18 ` [PATCH v5 5/7] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-05-19 18:18 ` [PATCH v5 6/7] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-05-19 18:18 ` [PATCH v5 7/7] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-05-19 19:23 ` [PATCH v5 0/7] scalar: implement the subcommand "diagnose" Junio C Hamano 2022-05-21 15:08 ` [PATCH v6 " Johannes Schindelin via GitGitGadget 2022-05-21 15:08 ` [PATCH v6 1/7] archive: optionally add "virtual" files Johannes Schindelin via GitGitGadget 2022-05-25 21:11 ` Junio C Hamano 2022-05-26 9:09 ` René Scharfe 2022-05-26 17:10 ` Junio C Hamano 2022-05-26 18:57 ` René Scharfe 2022-05-26 20:16 ` Junio C Hamano 2022-05-27 17:02 ` René Scharfe 2022-05-27 19:01 ` Junio C Hamano 2022-05-28 6:57 ` René Scharfe 2022-05-21 15:08 ` [PATCH v6 2/7] archive --add-virtual-file: allow paths containing colons Johannes Schindelin via GitGitGadget 2022-05-25 20:22 ` Junio C Hamano 2022-05-25 21:42 ` Junio C Hamano 2022-05-25 22:34 ` Junio C Hamano 2022-05-21 15:08 ` [PATCH v6 3/7] scalar: validate the optional enlistment argument Johannes Schindelin via GitGitGadget 2022-05-21 15:08 ` [PATCH v6 4/7] Implement `scalar diagnose` Johannes Schindelin via GitGitGadget 2022-05-21 15:08 ` [PATCH v6 5/7] scalar diagnose: include disk space information Johannes Schindelin via GitGitGadget 2022-05-21 15:08 ` [PATCH v6 6/7] scalar: teach `diagnose` to gather packfile info Matthew John Cheetham via GitGitGadget 2022-05-21 15:08 ` [PATCH v6 7/7] scalar: teach `diagnose` to gather loose objects information Matthew John Cheetham via GitGitGadget 2022-05-28 23:11 ` [PATCH v6+ 0/7] js/scalar-diagnose rebased Junio C Hamano 2022-05-28 23:11 ` [PATCH v6+ 1/7] archive: optionally add "virtual" files Junio C Hamano 2022-05-28 23:11 ` [PATCH v6+ 2/7] archive --add-virtual-file: allow paths containing colons Junio C Hamano 2022-06-15 18:16 ` Adam Dinwoodie 2022-06-15 20:00 ` Junio C Hamano 2022-06-15 21:36 ` Adam Dinwoodie 2022-06-18 20:19 ` Johannes Schindelin 2022-06-18 22:05 ` Junio C Hamano 2022-06-20 9:41 ` Adam Dinwoodie 2022-05-28 23:11 ` [PATCH v6+ 3/7] scalar: validate the optional enlistment argument Junio C Hamano 2022-05-28 23:11 ` [PATCH v6+ 4/7] scalar: implement `scalar diagnose` Junio C Hamano 2022-06-10 2:08 ` Ævar Arnfjörð Bjarmason 2022-06-10 16:44 ` Junio C Hamano 2022-06-10 17:35 ` Ævar Arnfjörð Bjarmason 2022-05-28 23:11 ` [PATCH v6+ 5/7] scalar diagnose: include disk space information Junio C Hamano 2022-05-28 23:11 ` [PATCH v6+ 6/7] scalar: teach `diagnose` to gather packfile info Junio C Hamano 2022-05-28 23:11 ` [PATCH v6+ 7/7] scalar: teach `diagnose` to gather loose objects information Junio C Hamano 2022-05-30 10:12 ` [PATCH v6+ 0/7] js/scalar-diagnose rebased Johannes Schindelin 2022-05-30 17:37 ` Junio C Hamano
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b49d396d-a433-51a4-2d19-55e175af571a@web.de \ --to=l.s.r@web.de \ --cc=Johannes.Schindelin@gmx.de \ --cc=git@vger.kernel.org \ --cc=gitgitgadget@gmail.com \ --cc=gitster@pobox.com \ --cc=me@ttaylorr.com \ --cc=newren@gmail.com \ --cc=stolee@gmail.com \ --subject='Re: [PATCH v2 1/6] archive: optionally add "virtual" files' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).