Am 08.02.22 um 18:44 schrieb Junio C Hamano: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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@xxxxxx> --- 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
