Re: git fails with control characters in trunk directory name
- From
Alex Riesen <raa.lkml@gmail.com>
- Date
- May 12, 2009, 14:59 UTC
- Message-ID
- <81b0412b0905120759u15f1ec73k73625a7904515792@mail.gmail.com>
- In-Reply-To
- <200905121557.18542.Hugo.Mildenberger@namir.de>
2009/5/12 Hugo Mildenberger <Hugo.Mildenberger@namir.de>:
Show 17 quoted lines
>> OTOH, a warning about commonly used delimiters not fitting a name >> context, maybe a good idea. Like "\r\n\t", backslash (came up recently >> on this list). Such a check and associated warning may be useful for >> repository names and branches. >> >> Still, it's more of a policy issue and I would make it optional, even >> if enabled by default. Maybe even by defining a regexp which the >> repo name or branch name must (for hard error) or should (for a warning) >> match. >> >> For your specific case, you can take a look at builtin-clone.c, >> just after the line containing "guess_dir_name(repo_name"... >> -- > > But at least the git versions I tried (up to 1.6.3) really do have a problem > when facing a trailing newline in repository names; so one should enforce a > convention.
That's what I mean by saying: "... even if enabled by default". I just want to disable newbie-helping annoyances on my systems.
> I looked into guess_dir_name().
That's not the right place. The place I meant is right below the call to this function (you have to parse the names given in the command-line too).
The automatically generated (that's the case with guess_dir_name) directory name certainly shouldn't contain any unexpected characters.
> A regex call would be easy to fit, but > currently the git binary does not depend on libpcre. Is it generally > considered to be acceptable to add such a dependency?
No. And pcre is not the only regex lib in the world. And we prefer shell patterns, if any at all.
> While I like the idea to make use of a configurable regular expression, such > an expression had to be a command line parameter with a reasonable default > value, because .git/config still would not exist when the value would be > needed.
That's where _default_ policy plays its role. "Default" like in "it is compiled into the git executable and needs no configuration present".
Show 6 quoted lines
> Last not least, I managed to reproduce the problem almost exactly: > > 1.) hm@localhost git > clone "git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git > " > (Note the trailing linefeed)
That's all the command printed? No "Initialized empty Git repository" line?
> 2.) hm@localhost ~/tmp/bluetooth-testing.git $ make
Hmm... At this point the clone may have worked (at least partially). It named "bluetooth-testing.git", which it shouldn't (but explainable: the repo url suffix is not .git anymore, but ".git\r\n"). But it looks like the post-clone checkout failed (silently? which would be bad):
Show 7 quoted lines
> Makefile:313: /home/hm/tmp/bluetooth-testing.git > /scripts/Kbuild.include: No such file or directory > make[1]: /home/hm/tmp/bluetooth-testing.git: No such file or directory > make[1]: *** No rule to make target `/home/hm/tmp/bluetooth-testing.git'. > Stop. > make: *** No rule to make target `include/config/auto.conf', needed by > `include/config/kernel.release'. Stop.
Assuming the files must be present, of course.
Show 10 quoted lines
> 3.) hm@localhost ~/tmp/bluetooth-testing.git $ git pull > fatal: Error in line 2: > > 4.) ".git/config" now contains > > url = > git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-testing.git\n > > I particulary liked the git message "fatal: Error in line 2:" ... >
Separate issue. Will look at it later.