René Scharfe <l.s.r@xxxxxx> writes: > Am 11.05.22 um 01:21 schrieb Junio C Hamano: >> <rsbecker@xxxxxxxxxxxxx> writes: >> >>>> If we did "--add-virtual-file=<path>:0644:<contents>" instead from day one, it >>>> certainly adds a few more lines of logic to this patch, and the calling "scalar >>>> diagnose" may have to pass a few more bytes, but I suspect that such a change >>>> would help the project in the longer run. > >> I did say "scalar diagnose" may not care. But a patch to "git >> archive" will affect other people, and among them there would be >> people who say "gee, now I can add a handful of files from the >> command line with their contents, without actually having them in >> throw-away untracked files, when running 'git archive'. That's >> handy!", try it out and get disappointed by their inability to >> create executable files that way. > > Which might motivate them to contribute a patch to add that feature. > Give them a chance! :) Yes, but there is no way to reuse the same option in a backward compatible way to later add the mode information, and that is why we want to be careful before a half-feature squats on an option. > FWIW, I'd already be satisfied by a convincing outline of a way towards > a complete solution to accept the partial feature, just to be sure we > don't paint ourselves into a corner. Exactly. As you say, an extra and separate option can be used. I do not know if that is a workaround because we didn't design the first option to take an additional option, or a welcome feature. > Regarding file modes: We only effectively support the executable bit, > so an additional option --add-virtual-executable-file=<path>:<contents> > would suffice. While I do not think we want to support more than one "is it executable or not?" bit, I am not so sure about what the current code does, though, for these "not from a tree, but added as extra files" entries. If you add an extra file from an on-disk untracked file, the add_file_cb() callback picks up the full st.st_mode for the file, and write_archive_entries() in its loop over args->extra_files pass the full info->stat.st_mode down to write_entry(), which is used by archive-tar.c::write_tar_entry() to obtain mode bits pretty much as-is. For tracked paths, we probably are normalizing the blobs between 0644 and 0755 way before the values are passed as "mode" parameter to the write_entry() functions, but for these extra files, there is no such massaging. So, I am OK with --add-virtual-executable=<path>:<contents> (but the point still stands that the way the code in the patch squats in the codepath makes it necessary to first refator it before it can happen) as a separate option. We may want to massage the mode bit we grab from these extra files, if we were to go that route, though. Thanks.
