Re: git fails with control characters in trunk directory name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2009/5/12 Hugo Mildenberger <Hugo.Mildenberger@xxxxxxxx>:
>> 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".

> 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):

>        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.

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]