Junio C Hamano wrote: > "Victoria Dye via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> Also, >> remove trailing slashes on directories before validating, allowing users to >> provide 'folder-name/' as the path for a tree object entry. > > Is that a good idea for a plumbing like this command? We would > silently accept these after silently stripping the trailing slash? > > 040000 tree 82a33d5150d9316378ef1955a49f2a5bf21aaeb2 templates/ > 100644 blob 1f89ffab4c32bc02b5d955851401628a5b9a540e thread-utils.c/ > > The former _might_ count as "usability improvement", but if we are > doing the same for the latter we might be going a bit too lenient. The trailing slashes are only ignored on tree entries (with mode 040000), so the latter case would not be allowed (and triggers a 'die()' as it would today). > > Let's see what really happens in the code. > >> @@ -49,10 +50,23 @@ static void append_to_tree(unsigned mode, struct object_id *oid, const char *pat >> { >> struct tree_entry *ent; >> size_t len = strlen(path); >> - if (!literally && strchr(path, '/')) >> - die("path %s contains slash", path); >> >> - FLEX_ALLOC_MEM(ent, name, path, len); >> + if (literally) { >> + FLEX_ALLOC_MEM(ent, name, path, len); >> + } else { >> + /* Normalize and validate entry path */ >> + if (S_ISDIR(mode)) { >> + while(len > 0 && is_dir_sep(path[len - 1])) >> + len--; >> + } > > Leave a single SP after "while", please. Ah, sorry about that, thanks for catching it. > We do this only to subtree entries, and all trailing slashes, not > just a single one. OK, but I am not sure if the extra leniency is a > good idea to begin with. "ls-tree" output does not have such a > trailing slashes, so it is unclear whom we are trying to be extra > nice with this. It might be a bit niche, but 'git ls-files -s --sparse' does print directories with a trailing slash, and in a format that is otherwise accepted by the command after switching to 'read_index_info' for input parsing.