Git development
 help / color / mirror / Atom feed
* Re: [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits
From: Jeff King @ 2011-10-19 19:01 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List
In-Reply-To: <CACBZZX5PqYa0uWiGgs952rk2cy+QRCU95kF63qzSi3fKK-YrCQ@mail.gmail.com>

On Wed, Oct 19, 2011 at 08:03:24PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This is quick hack I wrote just before leaving work to show that I
> could indeed push patches to our main repository starting with
> 31337. Names hidden to protect the innocent.

Clever and amusing.

> Which in just over a minute will generate, in my case:
> 
>     $ git show --pretty=raw 313375d995e6f8b7773c6ed1ee165e5a9e15690b | head -n 7
>     commit 313375d995e6f8b7773c6ed1ee165e5a9e15690b
>     tree c9bebc99c05dfe61cccf02ebdf442945c8ff8b3c
>     parent 0dce2d45a79d26a593f0e12301cdfeb7eb23c17a
>     author Ævar Arnfjörð Bjarmason <avar@example.com> <censored> <censored>
>     committer Ævar Arnfjörð Bjarmason <avar@example.com> <censored> <censored>
>     lulz 697889

Nice header name.

> I also think it's interesting that we seemingly don't have (in my
> brief search, maybe I missed it) an API for writing a complete commit
> object into a strbuf, so I had to write a lot of commit objects to
> disk, and keep unlinking the unacceptable candidates so the repository
> wouldn't balloon in size.

Calculating an object sha1 is pretty straightforward. I think you could
just tack the "commit <size>" header in front of the commit strbuf, and
sha1 that, and then just send the "winner" to actually be written.

Too bad it won't work in an append only way.  The internal state of sha1
after a certain set of bytes is deterministic, so you could do something
like:

  void leetify_object(struct strbuf *data)
  {
          SHA_CTX base, guess;
          int len = data->len;
          int i = 0;

          /* come up with a base internal state representing
           * the commit */
          SHA1_Init(&base);
          SHA1_Update(&base, data->buf, data->len);

          while (1) {
                  char append[64];
                  int n = snprintf(append, sizeof(append),
                                   "x-lulz-by: %d\n", i);
                  unsigned char sha1[20];

                  /* and then from that state, see what the sha1 would
                   * be if we add our few new bytes */
                  memcpy(&guess, &base, sizeof(guess));
                  SHA1_Update(&guess, append, n);
                  SHA1_Final(sha1, &guess);

                  /* did we pick a winner? */
                  if (sha1[0] == 0x31 &&
                      sha1[1] == 0x33 &&
                      &sha1[2] & 0xf0) == 0x70) {
                            strbuf_addstr(data, append);
                            return;
                  }

                  i++;
          }
  }

There are two problems with this:

  1. The x-lulz-by header is visible in the commit message. I think you
     can actually get around this by appending a NUL character, and "git
     log" and friends will stop processing the commit message there.

  2. The object header will actually have the length of the object at
     the beginning. So as your lulz number grows, the length of the
     object will change, and invalidate your earlier sha1. You could get
     around this by using a fixed-size guess (e.g., start with 20 bytes
     of zeroes, and then just keep incrementing).

-Peff

^ permalink raw reply

* [PATCH] git-show-ref: fix escaping in asciidoc source
From: mhagger @ 2011-10-19 18:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael Haggerty

From: Michael Haggerty <mhagger@alum.mit.edu>

One of the "^" characters was not coming through in the man page.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 Documentation/git-show-ref.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 3c45895..87f358d 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -44,7 +44,7 @@ OPTIONS
 -d::
 --dereference::
 
-	Dereference tags into object IDs as well. They will be shown with "^{}"
+	Dereference tags into object IDs as well. They will be shown with "{caret}\{\}"
 	appended.
 
 -s::
@@ -75,7 +75,7 @@ OPTIONS
 	Make 'git show-ref' act as a filter that reads refs from stdin of the
 	form "^(?:<anything>\s)?<refname>(?:{backslash}{caret}\{\})?$"
 	and performs the following actions on each:
-	(1) strip "^{}" at the end of line if any;
+	(1) strip "{caret}\{\}" at the end of line if any;
 	(2) ignore if pattern is provided and does not head-match refname;
 	(3) warn if refname is not a well-formed refname and skip;
 	(4) ignore if refname is a ref that exists in the local repository;
-- 
1.7.7

^ permalink raw reply related

* [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits
From: Ævar Arnfjörð Bjarmason @ 2011-10-19 18:03 UTC (permalink / raw)
  To: Git Mailing List

You do not need to read this. Really, stop now.

This is quick hack I wrote just before leaving work to show that I
could indeed push patches to our main repository starting with
31337. Names hidden to protect the innocent.

With a quick call to getenv() i'll be easy to supply a prefix with an
environment variable, so you can have d00dbabe, deadbeef, b00b
etc. commits as time allows.

    < cow-orker-1> avar: what do I have to do to have all my
                            commit ids begin with 31337?
    < cow-orker-2> cow-orker-1: just figure out SHA1 collision
    < cow-orker-2> well with MD5 it is "easy" to have
                   same-prefix collisions, so you can
                   append junk that will be ignored or
                   have no effect, and have the same MD5
                   as the shorter version
    <@avar> It actually wouldn't be too hard to make all your commits
begin with 31337
    <@avar> I might patch my git client to do that
    <@avar> Basically the sha1 is a function of the commit object /
message / top-level tree
    <@avar> and any compliant git client ignores headers it doesn't know about
    <@avar> so you can just add a header "lulz %d"
    <@avar> where %d is a number you keep incrementing until your
resulting sha1 happens to begin with 31337
    <@avar> 31337 is a 5 character hex string, and 16^5/2 = 524288
    <@avar> so you'd on average have to generate half a million
commits before you'd get the desired results
    <@avar> which would slow down git somewhat at commit time, but not
much more than using svn :)
    < cow-orker-2> enjoy your 3MB commits
    < cow-orker-2> ah you can do that on the same header
    <@avar> yes
    < cow-orker-2> thanks, you just ruined my evening
    [...]
    <@avar> Also you don't have to waste your evening, I've implemented this
    < cow-orker-1> \o/
    < cow-orker-3> hurry up and make a commit, already
    < cow-orker-3> [img-mj-popcorn.gif]
    <@avar> victory is mine: 313375d995e6f8b7773c6ed1ee165e5a9e15690b !

This is how you use it:

    $ time ~/g/git/git-commit -F /tmp/commit-message
    Try 0/4000000 to get a 1337 commit =
33d86a5a13ce07914a38ead3a517e391df0cc8c2
    Try 100000/4000000 to get a 1337 commit =
058e7663f54a3dd85e2c5a88cebf221ff7c25889
    Try 200000/4000000 to get a 1337 commit =
78839302cf449d088db1df5eb81f9116cbce55d0
    Try 300000/4000000 to get a 1337 commit =
f0af0cbe91d0ba8903085e2f867246095e6fb957
    Try 400000/4000000 to get a 1337 commit =
2068605f90321558e97ae5a083f63475ae2075ea
    Try 500000/4000000 to get a 1337 commit =
f8b4e2acd14ab111fd7957956368d181596bc6ef
    Try 600000/4000000 to get a 1337 commit =
3a479ab83d969f9b5638481445506036d1e1db46
    Try 700000/4000000 to get a 1337 commit =
29b98585b44c02e59240c1d8a1956ae434b3543f
    Try 800000/4000000 to get a 1337 commit =
8749e31c40200554b3758313cada1a3f596b0230
    Try 900000/4000000 to get a 1337 commit =
3fb2e617db95db8c027fba686c3f75eaa1b7f880
    commit id = 313375d995e6f8b7773c6ed1ee165e5a9e15690b
    [trunk 313375d] <censored>
     <censored>

    real    1m15.389s
    user    0m45.969s
    sys     0m29.368s

Which in just over a minute will generate, in my case:

    $ git show --pretty=raw 313375d995e6f8b7773c6ed1ee165e5a9e15690b | head -n 7
    commit 313375d995e6f8b7773c6ed1ee165e5a9e15690b
    tree c9bebc99c05dfe61cccf02ebdf442945c8ff8b3c
    parent 0dce2d45a79d26a593f0e12301cdfeb7eb23c17a
    author Ævar Arnfjörð Bjarmason <avar@example.com> <censored> <censored>
    committer Ævar Arnfjörð Bjarmason <avar@example.com> <censored> <censored>
    lulz 697889

And this is the evil one-off code for this:

    diff --git a/commit.c b/commit.c
    index 73b7e00..71d1605 100644
    --- a/commit.c
    +++ b/commit.c
    @@ -853,2 +853,4 @@ int commit_tree(const char *msg, unsigned char *tree,
            int encoding_is_utf8;
    +       int try;
    +       int tries = 4000000;
            struct strbuf buffer;
    @@ -857,2 +859,5 @@ int commit_tree(const char *msg, unsigned char *tree,

    +       struct commit_list *parents_ptr = parents;
    +
    +       for (try = 0; try < tries; try++) {
            /* Not having i18n.commitencoding is the same as having utf-8 */
    @@ -872,3 +877,2 @@ int commit_tree(const char *msg, unsigned char *tree,
                            sha1_to_hex(parents->item->object.sha1));
    -               free(parents);
                    parents = next;
    @@ -876,2 +880,4 @@ int commit_tree(const char *msg, unsigned char *tree,

    +               parents = parents_ptr;
    +
            /* Person/date information */
    @@ -883,2 +889,4 @@ int commit_tree(const char *msg, unsigned char *tree,
                    strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
    +
    +               strbuf_addf(&buffer, "lulz %d\n", try);
            strbuf_addch(&buffer, '\n');
    @@ -893,3 +901,21 @@ int commit_tree(const char *msg, unsigned char *tree,
            result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
    +
    +               if (result) {
    +                       die("failed to write commit object");
    +               } else {
    +                       if (strncmp(sha1_to_hex(ret), "31337", 5) == 0) {
    +                               printf("commit id = %s\n",
sha1_to_hex(ret));
    +                               goto done;
    +                       } else {
    +                               if (try % 100000 == 0) {
    +                                       fprintf(stderr, "Try %d/%d
to get a 1337 commit = %s\n", try, tries, sha1_to_hex(ret));
    +                               }
    +                               unlink_or_warn(sha1_file_name(ret));
    +                       }
            strbuf_release(&buffer);
    +               }
    +       }
    +       done:
    +       strbuf_release(&buffer);
    +
            return result;

There's nothing new or novel about this. I just thought it would be
nice to share it. Someone asked me if having a "lulz" header wouldn't
break things, but since we introduced the "encoding" header a while
back clients have learned to ignore unknown headers, so it doesn't.

Which is why we can discuss e.g. adding GPG headers without worrying
about breaking everything.

I also think it's interesting that we seemingly don't have (in my
brief search, maybe I missed it) an API for writing a complete commit
object into a strbuf, so I had to write a lot of commit objects to
disk, and keep unlinking the unacceptable candidates so the repository
wouldn't balloon in size.

^ permalink raw reply

* Re: [PATCH] t/t3000-ls-files-others.sh: use $SHELL_PATH to run git-new-workdir script
From: Junio C Hamano @ 2011-10-19 17:31 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Brandon Casey
In-Reply-To: <jKc1nei6yQLMU5upFxa60klqkQwEDsUHt5jcsbbnbL-TuvERAV3NOSvVH9yzlpgnPdDi0-5rPkBeDx7SQF7CEqlyj9UX6NqKccGu9kUyq1SFu2oCzI2xkRSoDmDBH66WbRZaTWHtj8ubURHpkyEvMA@cipher.nrlssc.navy.mil>

Thanks.

^ permalink raw reply

* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-19 17:10 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <4E9EEA4D.50207@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 10/19/2011 08:19 AM, Junio C Hamano wrote:
> 
>> A possible alternative might be to leave check_refname_format() and its
>> callers as they are, introduce check_full_refname() function that knows
>> the new restriction on top of check-ref-format-fixup, and use that in
>> lock_ref_sha1(), lock_any_ref_for_update() and is_refname_available()
>> [*2*]. That way, we can keep the potentially useful "ill-formed contents
>> in the ref" warning and avoid possible confusion caused by random files
>> that are directly under $GIT_DIR, which would be far more preferable in
>> the longer term.
>> 
>> Anybody wants to give it a try?
>
> I think that the refs/-or-ALL_CAPS test fits most naturally in
> check_refname_format(), controlled by an option flag.

That is fine too, if a separate function check_full_refname() would end up
being nothing more than a thin wrapper that passes the "we are checking a
full refname" option.

> I'm starting by building parts of the solution, namely something like:
> ...
>
> Hopefully I'll have patches before the end of the (Berlin) day.

It is unclear how these two functions could be related to the issue, but
hopefully we will soon find out when we see your patch.  Thanks.

^ permalink raw reply

* Re: How to verify that lines were only moved, not edited?
From: Junio C Hamano @ 2011-10-19 17:07 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4E9EDFEC.3040009@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> I thought there was a way to use git-blame to find out whether a change
> only shuffled lines, but otherwise did not modify them. I tried "git blame
> -M -- the/file",...

You said "a change" and I somehow expected that such a blame would be done
with a revision range, e.g. "git blame -M HEAD^..HEAD -- the/file".

If the two endpoints you are comparing have other commits in between that
make changes then revert them in such a way that the end result cancels
out, "git diff A B -- the/file" won't see such intermediate changes, but
they may interfere with "git blame A..B -- the/file", i.e. when A is not a
direct parent of B.

> ... nor with a 5000+ lines file (with 55 lines moved).

> ... while this produces the same as with just -M:
> $ git blame -M2 -s -- foo

Yes, blame tries to omit matches that consists only of non words, so that
you won't see "all those lines with a single "}" on them that close
definitions for your 100 new functions were copied from the closing brace
of one function you originally had in the file" symptom, and -M<level>
controls it.

> But neither helps with my 5000+ lines file. Does it mean that the lines
> were changed? But I'm sure they were just moved! Please help!

When reviewing a "supposedly move-only" change, I typically just grab +/-
blocks from the patch, remove the +/- prefix and run comparison between
them.

^ permalink raw reply

* Re: How to verify that lines were only moved, not edited?
From: Jeff King @ 2011-10-19 16:33 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4E9EDFEC.3040009@viscovery.net>

On Wed, Oct 19, 2011 at 04:34:20PM +0200, Johannes Sixt wrote:

> I thought there was a way to use git-blame to find out whether a change
> only shuffled lines, but otherwise did not modify them. I tried "git blame
> -M -- the/file", but it does not work as expected, neither with a toy file
> nor with a 5000+ lines file (with 55 lines moved).
> 
> git init
> echo A > foo
> echo B >> foo
> git add foo
> git commit -m initial
> echo B > foo
> echo A >> foo
> git commit -a -m swapped
> 
> The results are:
> $ git blame -M -s -- foo
> ^e3abca2 1) B
> 6189cb46 2) A
> 
> I would have expected:
> ^e3abca2 1) B
> ^e3abca2 2) A
> 
> Oh, look! This produces the expected result:
> $ git blame -M1 -s -- foo

Right. Your toy lines aren't long enough to be considered "interesting"
by the default score. From git-blame(1):

  -M[<num>]
  [...]
  <num> is optional but it is the lower bound on the number of
  alphanumeric characters that git must detect as moving/copying within
  a file for it to associate those lines with the parent commit. The
  default value is 20.

Whereas with a longer sample:

  git init
  seq 1 5000 >foo
  git add foo
  git commit -m initial
  sed -i '/^2..$/d' foo
  seq 200 299 >>foo
  git commit -a -m 'move 200-299 to end'

I get the expected result from "git blame -M" (i.e., everything
attributed to the root commit).

> But neither helps with my 5000+ lines file. Does it mean that the lines
> were changed? But I'm sure they were just moved! Please help!

What does the file look like? I think blame has some heuristics about
lines which are uninteresting, and maybe you are triggering a corner
case there.

-Peff

^ permalink raw reply

* [PATCH] t/t3000-ls-files-others.sh: use $SHELL_PATH to run git-new-workdir script
From: Brandon Casey @ 2011-10-19 16:26 UTC (permalink / raw)
  To: gitster; +Cc: git, Brandon Casey

From: Brandon Casey <drafnel@gmail.com>


Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t3000-ls-files-others.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index e9160df..88be904 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -77,7 +77,7 @@ test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
 	) &&
 	(
 		cd super &&
-		"$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub
+		"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub
 		git ls-files --others --exclude-standard >../actual
 	) &&
 	echo sub/ >expect &&
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH] t1300: attempting to remove a non-existent .git/config is not an error
From: Jeff King @ 2011-10-19 16:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Carlos Martín Nieto, git
In-Reply-To: <4E9E7E22.7010905@viscovery.net>

On Wed, Oct 19, 2011 at 09:37:06AM +0200, Johannes Sixt wrote:

> From: Johannes Sixt <j6t@kdbg.org>
> 
> Since some tests before test number 79 ("quoting") are skipped, .git/config
> does not exist and 'rm .git/config' fails. Fix this particular case.
> 
> While at it, move other instance of 'rm .git/config' that occur in this
> file inside the test function to document that the test cases want to
> protect themselves from remnants of earlier tests.

Thanks, this fix looks good to me.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Michael Haggerty @ 2011-10-19 15:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7vty75ec54.fsf@alter.siamese.dyndns.org>

On 10/19/2011 08:19 AM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> I was trying to summarize this topic for Release Notes.
>>
>>   Possibly incompatible changes
>>   -----------------------------
>>
>>    * Special refs such as "HEAD", "MERGE_HEAD", and "FETCH_HEAD" are now
>>      restricted to names with only uppercase letters and underscore. All
>>      other refs must live under refs/ namespace. Earlier, you could
>>      confuse git by storing an object name in $GIT_DIR/tmp/junk and say
>>      "git show tmp/junk", but this will no longer work.
>>
>> But noticed that "git update-ref tmp/junk HEAD" does create such a ref
>> that won't be recognized, and "git check-ref-format tmp/junk" is happy.
>>
>> I think we would need to restrict check_ref_format() so that these
>> commands (and possibly others, but I think that single function will cover
>> pretty much everything) also reject "tmp/junk" immediately below $GIT_DIR
>> as a bad string. Otherwise we cannot merge these fixups, which would mean
>> we would have to revert the "Clean up refname checks and normalization"
>> series, at least the part that started emitting the "warning", which is a
>> mess I would rather want to avoid.
>>
>> Opinions on how to get us out of this mess?
> 
> Addendum.
> 
> I was digging this further and see fairly large conflicts between the bulk
> of "clean up refname checks and normalization" topic and the logic to
> avoid the additional warning by tightening the dwimmery.
> 
> check_refname_format() has always assumed that it is OK to be called at
> any level of substring, and there are many code like this one (example is
> from remote.c::get_fetch_map()):
> 
>         for (rmp = &ref_map; *rmp; ) {
>                 if ((*rmp)->peer_ref) {
>                         if (check_refname_format((*rmp)->peer_ref->name + 5,
>                                 REFNAME_ALLOW_ONELEVEL)) {
>                                 struct ref *ignore = *rmp;
>                                 error("* Ignoring funny ref '%s' locally",
>                                       (*rmp)->peer_ref->name);
> 
> This code somehow _knows_ that peer_ref->name begins with "refs/" and that
> is the reason it adds 5 to skip that known part. In this particular case,
> I think we can simply drop the +5 and allow-onelevel, but there are other
> instances of the calls to the function that feeds the rest of the refname
> string, skipping leading substring (not necessarily "refs/"), assuming
> that any component string is either valid or invalid no matter where it
> appears in the full refname. I wouldn't be surprised if some callers do
> not even have enough information to tell what the leading substring would
> be in the full refname context (e.g. parsing of "master:master" refspec,
> relying on the later dwimmery to add refs/heads/ in front, could just
> verify that "master" is in good format with allow-onelevel).
> 
> The new restriction bolted on to that logic in jc/check-ref-format-fixup
> series to work around the new warning in 629cd3a (resolve_ref(): emit
> warnings for improperly-formatted references, 2011-09-15) is incompatible
> with the assumption, as we would need to check full refname, and treat the
> first refname component very differently from other components. It has to
> be either "refs" in multi-component refname, or all caps in a one-level
> one, but the callers of check_refname_format() are not designed to feed
> the full refname to begin with.
> 
> I am tempted to revert 629cd3a (resolve_ref(): emit warnings for
> improperly-formatted references, 2011-09-15) that started giving the
> warnings, and drop the jc/check-ref-format-fixup topic [*1*] altogether,
> but that is a short-sighted workaround I would rather want to avoid. It
> essentially declares that the "Clean up refname checks" topic was a
> failure and did not manage to really clean things up.
> 
> A possible alternative might be to leave check_refname_format() and its
> callers as they are, introduce check_full_refname() function that knows
> the new restriction on top of check-ref-format-fixup, and use that in
> lock_ref_sha1(), lock_any_ref_for_update() and is_refname_available()
> [*2*]. That way, we can keep the potentially useful "ill-formed contents
> in the ref" warning and avoid possible confusion caused by random files
> that are directly under $GIT_DIR, which would be far more preferable in
> the longer term.
> 
> Anybody wants to give it a try?

I think that the refs/-or-ALL_CAPS test fits most naturally in
check_refname_format(), controlled by an option flag.

I'm starting by building parts of the solution, namely something like:

* Add an option REFNAME_ALLOW_DWIM which causes check_refname_format()
to accept reference names that *could* be dwimmed into a valid refname.
 Specifically, when this option is *not* specified, then the option must
start with "refs/" or be ALL_CAPS.  When it *is* specified, the behavior
will be much like the current behavior when ALLOW_ONELEVEL is specified
(in fact, the new option might make ALLOW_ONELEVEL obsolete).

* Add a new function parse_refname_prefix(str, len, flags) which tries
to read a refname from the front of str (of length len, not necessarily
NUL-terminated) and returns the length of the part that could be used.
This function will support the same flag argument as
check_refname_format() (in fact the latter would become a trivial
wrapper of the new function).  I expect that this function will be
useful for dwimmery.

Hopefully I'll have patches before the end of the (Berlin) day.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: git-gui: still smartcase (and pull into git.git)
From: Bert Wesarg @ 2011-10-19 14:59 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Git Mailing List, Andrew Ardill, Junio C Hamano

[ Cc'ing git@vger, because its not that private anymore ;-) ]

On Wed, Oct 19, 2011 at 15:06, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
>
> I've applied the last of your set of patches to the git-gui repository
> now. The search series with the regexp and smartcase search. I added
> themed entry fields using similar code to that I posted but also
> handling xp/vista and dealing with non-themed Tk.
>
> I had to add a trap to deal with partial regexp's. If you tried to
> enable regexp and then entered [Cc]opyrigth it would raise an error
> after opening the brace as its not a valid regexp. Handled by catching
> and returning {} which yields a pink background until the expression
> becomes valid again which I think works well.

Thanks for catching this.

>
> As I don't like the smartcase mode I've added a commit to only enable it
> if gui.search.smartcase is enabled. It looks like this was your
> intention anyway but the mode was always enabled. What I don't like
> about it is that you can't uncheck the Case checkbutton if there is a
> capital letter in the search box.

I checked my inspiration for this, it's a patch to the NEdit editor
for it's incremental searchbar, and you can't uncheck the Case
checkbutton there too, if an capital letter is entered. Other
smart-case implementation I know of, aren't incremental, for example
less with it's -i option. So duno what should happen. It does
definitive makes no sense to enter 'giT' and searching
case-insensitive, in my opinion.

I also have incorporated Andrew's suggestion to reset the case flag
when the user cleared the entry. Unfortunately, I can't send an
updated patch currently, maybe tomorrow, here is just the commit,
before fixing up the patch, which is now in your master.

 commit af9e02860629f9da20a9434bfec74c115916f4e2
Author: Bert Wesarg <bert.wesarg@googlemail.com>
Date:   Tue Oct 18 19:43:44 2011 +0200

    searchbar: reset case flag after the user cleared the text

diff --git a/lib/search.tcl b/lib/search.tcl
index 58e6852..3388eb9 100644 lib/search.tcl
--- a/lib/search.tcl
+++ b/lib/search.tcl
@@ -155,10 +155,10 @@ method _incrsearch {} {
 	if {[catch {$ctext index anchor}]} {
 		$ctext mark set anchor [_get_new_anchor $this]
 	}
-	if {[regexp {[[:upper:]]} $searchstring]} {
-		set casesensitive 1
-	}
 	if {$searchstring ne {}} {
+		if {[regexp {[[:upper:]]} $searchstring]} {
+			set casesensitive 1
+		}
 		set here [_do_search $this anchor mlen]
 		if {$here ne {}} {
 			$ctext see $here
@@ -169,6 +169,9 @@ method _incrsearch {} {
 		} else {
 			$w.ent configure -background lightpink
 		}
+	} else {
+		# reset case sensitivity, when the user cleared the entry field
+		set casesensitive $default_casesensitive
 	}
 }

This should probably be now guarded by the new smartcase flag.

On the other hand, maybe we should change the 'gui.search.smartcase'
into 'gui.search.case' with 3 values case/nocase/smart?

>
> All pushed to master now at repo.or.cz. We should see it all end up in
> 1.7.8 I reckon.

But I saw that Juno has already pulled your gitgui-0.15.0 tag, which
was at 843d6597 at that time, and now you moved the tag :(

Bert

>
> Thanks,
> --
> Pat Thoyts                            http://www.patthoyts.tk/
> PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD
>

^ permalink raw reply related

* How to verify that lines were only moved, not edited?
From: Johannes Sixt @ 2011-10-19 14:34 UTC (permalink / raw)
  To: git

I thought there was a way to use git-blame to find out whether a change
only shuffled lines, but otherwise did not modify them. I tried "git blame
-M -- the/file", but it does not work as expected, neither with a toy file
nor with a 5000+ lines file (with 55 lines moved).

git init
echo A > foo
echo B >> foo
git add foo
git commit -m initial
echo B > foo
echo A >> foo
git commit -a -m swapped

The results are:
$ git blame -M -s -- foo
^e3abca2 1) B
6189cb46 2) A

I would have expected:
^e3abca2 1) B
^e3abca2 2) A

Oh, look! This produces the expected result:
$ git blame -M1 -s -- foo

while this produces the same as with just -M:
$ git blame -M2 -s -- foo

But neither helps with my 5000+ lines file. Does it mean that the lines
were changed? But I'm sure they were just moved! Please help!

-- Hannes

^ permalink raw reply

* Re: What should "git fetch origin +next" should do?
From: Marc Branchaud @ 2011-10-19 13:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v8vohfvqs.fsf@alter.siamese.dyndns.org>

On 11-10-19 12:31 AM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> In all cases if the command-line refspec has no RHS then git should try to
>> figure out which local ref to update from the config, and it should die if it
>> can't figure out a local ref to create or update.  (As I said above, maybe
>> allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
>> into FETCH_HEAD.)
> 
> I'd agree with everything you said in your message, except for the above
> "it should die" part.
> 
> You are assuming that the user knows what his configured refspecs would
> normally do and that is the whole reason why "git fetch origin next" that
> would update the remote tracking branch specified for the upstream's next
> might feel more natural than the current behaviour. I too think that is a
> reasonable assumption.
> 
> With the same assumption, if you said "git fetch origin frotz" when you
> know you are not usually tracking the remote 'frotz' branch, the command
> just should store what is fetched in FETCH_HEAD (and nowhere else) without
> dying. I do not think how it helps the user to die in that situation.

Sounds reasonable to me.

In all cases, I'd expect fetch to tell me what it did.

>> All this gets a bit more complicated if the user has currently checked out
>> the a ref that should be updated (regardless of the presence of a LHS +).
> 
> That "do not update the currently checked out branch" already exists and
> is correctly handled by "git fetch", I think.

Sweet!

(I'd also be happy if fetch updated the ref, and left the checked-out HEAD
detached, with attendant messages.  But then I'm quite comfortable working
with detached HEADs.)

		M.

^ permalink raw reply

* shallow&single-branch clone?
From: Norbert Nemec @ 2011-10-19 13:30 UTC (permalink / raw)
  To: git

Hi there,

it seems like there is no way to clone a single branch with truncated 
history.

Truncating history is done by 'git clone --depth 1', there is not way to 
restrict 'clone' to a single branch (the --branch option still downloads 
all branches and only then chooses something other than HEAD as active 
branch).

The manual sequence
	git init
	git remote add -t master -f origin URL
	git checkout
allows a clone of a single branch but offers no means to truncate history.

The least intrusive solution would be an additional option to clone, 
perhaps '--branch-only'.

More user friendly, this options should be on by default when --depth is 
set. After all: who would expect branches to be cloned when the history 
is explicitely truncated?

Ideas?

Greetings,
Norbert Nemec

^ permalink raw reply

* Re: Problems after deleting submodule
From: Howard Miller @ 2011-10-19 10:23 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git
In-Reply-To: <4E9DD9E0.80808@web.de>

On 18 October 2011 20:56, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 18.10.2011 13:54, schrieb Howard Miller:
>> I included a submodule in my project then decided I didn't like
>> submodules and deleted it again. I followed the advice of delting
>> .gitmodules, the bit from .git/config and then git rm'ing the
>> submodule. Seemed to work. I then copied files with the same directory
>> name into where the submodule was. However, I can't add them.
>>
>> Doing git add /path/to/old/submodule - does nothing, files are not
>> staged, no error messages no nothing.
>> If I try to git rm /path/to/old/submodule - it just says 'did not
>> match any files'.
>>
>> It simply does not seem to want to add anything to the old submodule
>> location. I had a grep around and could not see any obvious references
>> in the repo.
>>
>> I'm a bit stuck... any suggestions for things I can try much appreciated.
>
> Looks like the gitlink entry of the submodule is still there. I assume
>        git ls-files --stage | grep 160000
> still shows the submodule? That would make it impossible to add anything
> in the former submodule directory.
>
> When I delete .gitmodules and the .git/config entry and do a
>        git rm sub/
> I get
>        fatal: pathspec 'sub/' did not match any files
>
> If I omit the trailing slash it is:
>        fatal: git rm: 'sub': Is a directory
>
> I have to do a
>        rm -rf sub
> followed by
>        git rm sub
> to get everything cleaned up.
>
> Does that help you?

Yes - that was exactly the problem. I'm slightly embarrassed I missed
that. For some reason the initial delete of the submodule left the
empty directory in place (could have been a mistake on my part). I
didn't notice when I copied the new files back but it was enough to
break it.

I can't help thinking there could be some warning when doing an 'add'
but I don't think I have my head around it enough :)

Anyway, thanks for the help (again) everyone!

^ permalink raw reply

* Re: Compiling on Windows
From: Vincent van Ravesteijn @ 2011-10-19  7:49 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: git
In-Reply-To: <CAH5451=7Em7sPzknVx8i2VBSAZxZwg1Awr8s3Nr2W=A6SDEZEw@mail.gmail.com>

Op 18-10-2011 6:08, Andrew Ardill schreef:
> Hi list, I have been searching for details on what is required to
> compile on Windows, but haven't found anything conclusive. Perhaps
> there is something on the wiki, but unfortunately it is down at the
> moment.
>
> Can anyone point me in the right direction? I would like to be able to
> compile and test topic branches, and perhaps even do some dev work on
> my windows machine.

I once wrote a little step-by-step tutorial on how to compile the native 
Windows Git with MSVC (Express).

http://blog.vfrconsultancy.nl/#post0

Be aware that git runs without apparent problems, but that all 
functionality written in shell scripts can't be used.

HTH,

Vincent

^ permalink raw reply

* [PATCH] t1300: attempting to remove a non-existent .git/config is not an error
From: Johannes Sixt @ 2011-10-19  7:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Carlos Martín Nieto, git
In-Reply-To: <7vzkgxcviz.fsf@alter.siamese.dyndns.org>

From: Johannes Sixt <j6t@kdbg.org>

Since some tests before test number 79 ("quoting") are skipped, .git/config
does not exist and 'rm .git/config' fails. Fix this particular case.

While at it, move other instance of 'rm .git/config' that occur in this
file inside the test function to document that the test cases want to
protect themselves from remnants of earlier tests.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 You may think that the justification of this change would be the
 other way round: protect test cases, then incidentally this fixes
 a failure on Windows. But for me it is as described: The motivation
 is the fix for Windows, and without that, the "protect test cases"
 part would not have happened. :-)

 t/t1300-repo-config.sh |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index fba5ae0..51caff0 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -558,8 +558,6 @@ EOF
 test_expect_success "section was removed properly" \
 	"test_cmp expect .git/config"
 
-rm .git/config
-
 cat > expect << EOF
 [gitcvs]
 	enabled = true
@@ -570,6 +568,7 @@ EOF
 
 test_expect_success 'section ending' '
 
+	rm -f .git/config &&
 	git config gitcvs.enabled true &&
 	git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
 	git config gitcvs.dbname %Ggitcvs2.%a.%m.sqlite &&
@@ -642,8 +641,6 @@ test_expect_success 'invalid bool (set)' '
 
 	test_must_fail git config --bool bool.nobool foobar'
 
-rm .git/config
-
 cat > expect <<\EOF
 [bool]
 	true1 = true
@@ -658,6 +655,7 @@ EOF
 
 test_expect_success 'set --bool' '
 
+	rm -f .git/config &&
 	git config --bool bool.true1 01 &&
 	git config --bool bool.true2 -1 &&
 	git config --bool bool.true3 YeS &&
@@ -668,8 +666,6 @@ test_expect_success 'set --bool' '
 	git config --bool bool.false4 FALSE &&
 	cmp expect .git/config'
 
-rm .git/config
-
 cat > expect <<\EOF
 [int]
 	val1 = 1
@@ -679,13 +675,12 @@ EOF
 
 test_expect_success 'set --int' '
 
+	rm -f .git/config &&
 	git config --int int.val1 01 &&
 	git config --int int.val2 -1 &&
 	git config --int int.val3 5m &&
 	cmp expect .git/config'
 
-rm .git/config
-
 cat >expect <<\EOF
 [bool]
 	true1 = true
@@ -699,6 +694,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success 'get --bool-or-int' '
+	rm -f .git/config &&
 	(
 		echo "[bool]"
 		echo true1
@@ -718,7 +714,6 @@ test_expect_success 'get --bool-or-int' '
 
 '
 
-rm .git/config
 cat >expect <<\EOF
 [bool]
 	true1 = true
@@ -732,6 +727,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success 'set --bool-or-int' '
+	rm -f .git/config &&
 	git config --bool-or-int bool.true1 true &&
 	git config --bool-or-int bool.false1 false &&
 	git config --bool-or-int bool.true2 yes &&
@@ -742,8 +738,6 @@ test_expect_success 'set --bool-or-int' '
 	test_cmp expect .git/config
 '
 
-rm .git/config
-
 cat >expect <<\EOF
 [path]
 	home = ~/
@@ -752,6 +746,7 @@ cat >expect <<\EOF
 EOF
 
 test_expect_success NOT_MINGW 'set --path' '
+	rm -f .git/config &&
 	git config --path path.home "~/" &&
 	git config --path path.normal "/dev/null" &&
 	git config --path path.trailingtilde "foo~" &&
@@ -800,7 +795,7 @@ cat > expect << EOF
 	hash = "test#test"
 EOF
 test_expect_success 'quoting' '
-	rm .git/config &&
+	rm -f .git/config &&
 	git config quote.leading " test" &&
 	git config quote.ending "test " &&
 	git config quote.semicolon "test;test" &&
-- 
1.7.7.1507.g94722

^ permalink raw reply related

* Re: [PATCH 1/2] t1300: put git invocations inside test function
From: Junio C Hamano @ 2011-10-19  7:04 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jeff King, Carlos Martín Nieto, git
In-Reply-To: <4E9E7115.60303@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> This innocently looking hunk fails on Windows, because the preceding tests
> are skipped, and .git/config does not exist. I was tempted to just change
> this to 'rm -f'. But there are a few other instances of 'rm .git/config'
> in this file that were *not* moved inside the test function.
>
> How would you like this solved?
>
> - Move this one out again
> - Add -f to just this one
> - Add -f everywhere
> - a combination of the above?

Probably "rm -f .git/config" to all tests would be the most appropriate,
as the desire to start from an empty config file is a sign that each test
wants to be as independent as possible from previous tests.

An alternative approach to achieve the independence would be to make each
test responsible for cleaning up after itself, with test_when_finished,
but that won't work for the first one, as the test framework would give
you the original one. Besides, that's trying to be cooperative when each
test can fail in an unexpected way (after all the purpose of tests is to
fail when things go wrong). In order to achieve isolation, each test
protecting themselves from others, as long as it can be easily and
reasonably done (e.g. a single "rm -f"), is probably far more preferrable
than each test trying to clean after itself, risking possible failure to
do so.

^ permalink raw reply

* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19  6:55 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Andrea Gelmini, git
In-Reply-To: <7v8vohebhx.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
> In addition, you must be careful about what is added with add_extra_ref();
> they are often not refs but are placeholders for Git to know that the
> history leading to it is complete, even though they do not exist as
> refs. The values of real refs that exist in your alternate object store
> are treated this way, because you know you do not have to fetch (if you
> are initiating fetch-pack) or receive (if the other side is initiating
> send-pack) histories leading to them from the other side.

I think a quick and simple rule would be that add_extra_ref() is to give
our history in the object database extra anchor points to avoid fetching
or receiving pushes of unnecessary history into our object database, when
we know our object database has certain histories that are not reachable
from our own refs available. The names given to add_extra_ref() would not
follow any normal refname rules (in fact, "refs/tags/v2.6.12-rc3^{}"
peeled notation was designed not to collide with real refs, so was ".have"
sent from receive-pack.c to send-pack.c even though the latter is not kept
track of with the refs mechanism).

They do not have to be shown when resolve_ref() is called. They only need
to appear in for_each_ref() so that revision walking machinery can use
them when we need to compute the set-difference of commits between what we
have and what the other side has.

Hope this clears things up a bit.

^ permalink raw reply

* Re: [PATCH 1/2] t1300: put git invocations inside test function
From: Johannes Sixt @ 2011-10-19  6:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Carlos Martín Nieto, Junio C Hamano, git
In-Reply-To: <20111012182920.GA18948@sigill.intra.peff.net>

Am 10/12/2011 20:29, schrieb Jeff King:
> @@ -750,13 +759,6 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' '
>  	test_cmp expect result
>  '
>  
> -rm .git/config
> -
> -git config quote.leading " test"
> -git config quote.ending "test "
> -git config quote.semicolon "test;test"
> -git config quote.hash "test#test"
> -
>  cat > expect << EOF
>  [quote]
>  	leading = " test"
> @@ -764,8 +766,14 @@ cat > expect << EOF
>  	semicolon = "test;test"
>  	hash = "test#test"
>  EOF
> -
> -test_expect_success 'quoting' 'cmp .git/config expect'
> +test_expect_success 'quoting' '
> +	rm .git/config &&
> +	git config quote.leading " test" &&
> +	git config quote.ending "test " &&
> +	git config quote.semicolon "test;test" &&
> +	git config quote.hash "test#test" &&
> +	test_cmp expect .git/config
> +'

This innocently looking hunk fails on Windows, because the preceding tests
are skipped, and .git/config does not exist. I was tempted to just change
this to 'rm -f'. But there are a few other instances of 'rm .git/config'
in this file that were *not* moved inside the test function.

How would you like this solved?

- Move this one out again
- Add -f to just this one
- Add -f everywhere
- a combination of the above?

-- Hannes

^ permalink raw reply

* Re: [PATCH 2/2] daemon: report permission denied error to clients
From: Clemens Buchacher @ 2011-10-19  6:33 UTC (permalink / raw)
  To: Junio C Hamano, Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <20111018204101.GB2072@ecki>

Clemens Buchacher <drizzd@aon.at> wrote:
>
> I guess if permission is denied for access over git://, then nobody
> can use the repository. So it's clearly a server side issue.
> 
> This change probably makes more sense for local access and over
> ssh. I already have a similar patch brewing for that.

As far as security is concerned, we have to treat ssh the same as git://, unless the user has permission to execute arbitrary commands and not just git-upload-pack. But I can think of no way to figure that out on the server side.

^ permalink raw reply

* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19  6:33 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Andrea Gelmini, git
In-Reply-To: <7vipnlebwb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a	refs/tags/v2.6.12-rc3
>>> a2755a80f40e5794ddc20e00f781af9d6320fafb	refs/tags/v2.6.12-rc3^{}
>>> [...]
>>>
>>> I've never seen this format before; is this the remote protocol for
>>> peeled refs or maybe the behavior of an old version of git?
>>
>> This should be very well documented and has been the output from fairly
>> early days of ls-remote.
>
> I take the first half back. The ls-remote documentation seems to have
> bit-rotten quite a bit.
>
> IIRC, we started doing this when we introduced auto-following of the tag
> objects. Even update-server-info knows about it so it way predates smart
> HTTP and is a fairly old and established behaviour.

In addition, you must be careful about what is added with add_extra_ref();
they are often not refs but are placeholders for Git to know that the
history leading to it is complete, even though they do not exist as
refs. The values of real refs that exist in your alternate object store
are treated this way, because you know you do not have to fetch (if you
are initiating fetch-pack) or receive (if the other side is initiating
send-pack) histories leading to them from the other side.

^ permalink raw reply

* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19  6:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Haggerty, Andrea Gelmini, git
In-Reply-To: <7vpqhtec2n.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a	refs/tags/v2.6.12-rc3
>> a2755a80f40e5794ddc20e00f781af9d6320fafb	refs/tags/v2.6.12-rc3^{}
>> [...]
>>
>> I've never seen this format before; is this the remote protocol for
>> peeled refs or maybe the behavior of an old version of git?
>
> This should be very well documented and has been the output from fairly
> early days of ls-remote.

I take the first half back. The ls-remote documentation seems to have
bit-rotten quite a bit.

IIRC, we started doing this when we introduced auto-following of the tag
objects. Even update-server-info knows about it so it way predates smart
HTTP and is a fairly old and established behaviour.

^ permalink raw reply

* Re: Git --reference bug(?)
From: Junio C Hamano @ 2011-10-19  6:21 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Andrea Gelmini, gitster, git
In-Reply-To: <4E9E59A7.7070307@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 10/19/2011 12:04 AM, Andrea Gelmini wrote:
>> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> /tmp/3.1 --reference /home/gelma/dev/kernel/linus/
>> Cloning into /tmp/3.1...
>> fatal: Reference has invalid format: 'refs/tags/3.1.1.1^{}'
>> fatal: The remote end hung up unexpectedly
>
> The upstream repo reports what look like non-reference references:
>
> $ git ls-remote --tags
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git | head
> 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11
> c39ae07f393806ccf406ef966e9a15afc43cc36a	refs/tags/v2.6.11^{}
> 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11-tree
> c39ae07f393806ccf406ef966e9a15afc43cc36a	refs/tags/v2.6.11-tree^{}
> 26791a8bcf0e6d33f43aef7682bdb555236d56de	refs/tags/v2.6.12
> 9ee1c939d1cb936b1f98e8d81aeffab57bae46ab	refs/tags/v2.6.12^{}
> 9e734775f7c22d2f89943ad6c745571f1930105f	refs/tags/v2.6.12-rc2
> 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2	refs/tags/v2.6.12-rc2^{}
> 0397236d43e48e821cce5bbe6a80a1a56bb7cc3a	refs/tags/v2.6.12-rc3
> a2755a80f40e5794ddc20e00f781af9d6320fafb	refs/tags/v2.6.12-rc3^{}
> [...]
>
> I've never seen this format before; is this the remote protocol for
> peeled refs or maybe the behavior of an old version of git?

This should be very well documented and has been the output from fairly
early days of ls-remote.

^ permalink raw reply

* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-19  6:19 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7v39epft32.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I was trying to summarize this topic for Release Notes.
>
>   Possibly incompatible changes
>   -----------------------------
>
>    * Special refs such as "HEAD", "MERGE_HEAD", and "FETCH_HEAD" are now
>      restricted to names with only uppercase letters and underscore. All
>      other refs must live under refs/ namespace. Earlier, you could
>      confuse git by storing an object name in $GIT_DIR/tmp/junk and say
>      "git show tmp/junk", but this will no longer work.
>
> But noticed that "git update-ref tmp/junk HEAD" does create such a ref
> that won't be recognized, and "git check-ref-format tmp/junk" is happy.
>
> I think we would need to restrict check_ref_format() so that these
> commands (and possibly others, but I think that single function will cover
> pretty much everything) also reject "tmp/junk" immediately below $GIT_DIR
> as a bad string. Otherwise we cannot merge these fixups, which would mean
> we would have to revert the "Clean up refname checks and normalization"
> series, at least the part that started emitting the "warning", which is a
> mess I would rather want to avoid.
>
> Opinions on how to get us out of this mess?

Addendum.

I was digging this further and see fairly large conflicts between the bulk
of "clean up refname checks and normalization" topic and the logic to
avoid the additional warning by tightening the dwimmery.

check_refname_format() has always assumed that it is OK to be called at
any level of substring, and there are many code like this one (example is
from remote.c::get_fetch_map()):

        for (rmp = &ref_map; *rmp; ) {
                if ((*rmp)->peer_ref) {
                        if (check_refname_format((*rmp)->peer_ref->name + 5,
                                REFNAME_ALLOW_ONELEVEL)) {
                                struct ref *ignore = *rmp;
                                error("* Ignoring funny ref '%s' locally",
                                      (*rmp)->peer_ref->name);

This code somehow _knows_ that peer_ref->name begins with "refs/" and that
is the reason it adds 5 to skip that known part. In this particular case,
I think we can simply drop the +5 and allow-onelevel, but there are other
instances of the calls to the function that feeds the rest of the refname
string, skipping leading substring (not necessarily "refs/"), assuming
that any component string is either valid or invalid no matter where it
appears in the full refname. I wouldn't be surprised if some callers do
not even have enough information to tell what the leading substring would
be in the full refname context (e.g. parsing of "master:master" refspec,
relying on the later dwimmery to add refs/heads/ in front, could just
verify that "master" is in good format with allow-onelevel).

The new restriction bolted on to that logic in jc/check-ref-format-fixup
series to work around the new warning in 629cd3a (resolve_ref(): emit
warnings for improperly-formatted references, 2011-09-15) is incompatible
with the assumption, as we would need to check full refname, and treat the
first refname component very differently from other components. It has to
be either "refs" in multi-component refname, or all caps in a one-level
one, but the callers of check_refname_format() are not designed to feed
the full refname to begin with.

I am tempted to revert 629cd3a (resolve_ref(): emit warnings for
improperly-formatted references, 2011-09-15) that started giving the
warnings, and drop the jc/check-ref-format-fixup topic [*1*] altogether,
but that is a short-sighted workaround I would rather want to avoid. It
essentially declares that the "Clean up refname checks" topic was a
failure and did not manage to really clean things up.

A possible alternative might be to leave check_refname_format() and its
callers as they are, introduce check_full_refname() function that knows
the new restriction on top of check-ref-format-fixup, and use that in
lock_ref_sha1(), lock_any_ref_for_update() and is_refname_available()
[*2*]. That way, we can keep the potentially useful "ill-formed contents
in the ref" warning and avoid possible confusion caused by random files
that are directly under $GIT_DIR, which would be far more preferable in
the longer term.

Anybody wants to give it a try?


[Footnote]

*1* The topic is misnamed; it is about fixing dwimmery, not checking.

*2* This currently does not seem to check the well-formedness of the ref
it is creating at all; it should check the "ref", but not "oldref" to
allow renaming a malformed ref to correct earlier mistakes.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox