Git development
 help / color / mirror / Atom feed
* Re: Python extension commands in git - request for policy change
From: Magnus Bäck @ 2012-11-27 14:35 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Michael Haggerty, Eric S. Raymond, git
In-Reply-To: <CAMP44s0WYiV3hTE7u28_Wd59FkGfu3o_psS0gocpnibzN4--Fg@mail.gmail.com>

On Sunday, November 25, 2012 at 06:40 EST,
     Felipe Contreras <felipe.contreras@gmail.com> wrote:

> On Sun, Nov 25, 2012 at 11:44 AM, Michael Haggerty
> <mhagger@alum.mit.edu> wrote:

[...]

> > On the contrary, there is *constant* traffic on the mailing list
> > about incompatibilities between different shell implementations (sh,
> > dash, bash, etc), not to mention those in other utilities (sed,
> > grep, etc) that one is forced to work with in shell scripts.
> > Compatibility is a *huge* pain when developing shell code for git.
> > The fact that users typically don't encounter such problems is due
> > to the hard work of POSIX lawyers on the mailing list correcting the
> > compatibility errors of mortal programmers.
>
> *Theoretical* incompatibilities on probably obscure systems. *I* have
> never seen such compatibility issues *in practice*.

While "constant traffic" probably overstates the issue, these are not
theoretical problems. I recall at least three cases in the last year
or so where Git has seen breakage with Solaris or Mac OS X because
of sed or tr incompatibilities, and I don't even read this list that
thoroughly.

[...]

-- 
Magnus Bäck
baeck@google.com

^ permalink raw reply

* Re: Re: Python extension commands in git - request for policy change
From: Johannes Schindelin @ 2012-11-27 15:33 UTC (permalink / raw)
  To: David Aguilar; +Cc: Felipe Contreras, esr, Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <CAJDDKr4cr3VXqx=CXgXSQrVTSjE=f=55HZns-xfNziJOXb3Vsw@mail.gmail.com>

Hi David,

On Mon, 26 Nov 2012, David Aguilar wrote:

> *cough* git-cola *cough*

If you had a couple of free cycles to help us get Python/Qt compiled in
msysGit, I will be happy to make a Git for Windows package including
git-cola.

Ciao,
Dscho

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* [PATCH] Extend runtime prefix computation
From: Michael Weiser @ 2012-11-27 16:30 UTC (permalink / raw)
  To: git

Support determining the binaries' installation path at runtime even if
called without any path components (i.e. via search path). Implement
fallback to compiled-in prefix if determination fails or is impossible.

Signed-off-by: Michael Weiser <weiser@science-computing.de>
---
- Has two very minor memory leaks - function is called only once per
  program execution. Do we care? Alternative: Use static buffer instead.

 exec_cmd.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 125fa6f..d50d7f8 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -4,28 +4,22 @@
 #define MAX_ARGS	32
 
 static const char *argv_exec_path;
-static const char *argv0_path;
+static const char *argv0_path = NULL;
 
 const char *system_path(const char *path)
 {
-#ifdef RUNTIME_PREFIX
-	static const char *prefix;
-#else
 	static const char *prefix = PREFIX;
-#endif
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
 		return path;
 
 #ifdef RUNTIME_PREFIX
-	assert(argv0_path);
-	assert(is_absolute_path(argv0_path));
-
-	if (!prefix &&
-	    !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
-	    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
-	    !(prefix = strip_path_suffix(argv0_path, "git"))) {
+	if (!argv0_path ||
+	    !is_absolute_path(argv0_path) ||
+	    (!(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
+	     !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
+	     !(prefix = strip_path_suffix(argv0_path, "git")))) {
 		prefix = PREFIX;
 		trace_printf("RUNTIME_PREFIX requested, "
 				"but prefix computation failed.  "
@@ -41,20 +35,64 @@ const char *system_path(const char *path)
 const char *git_extract_argv0_path(const char *argv0)
 {
 	const char *slash;
+	char *abs_argv0 = NULL;
 
 	if (!argv0 || !*argv0)
 		return NULL;
 	slash = argv0 + strlen(argv0);
 
+	/* walk to the first slash from the end */
 	while (argv0 <= slash && !is_dir_sep(*slash))
 		slash--;
 
+	/* if there was a slash ... */
 	if (slash >= argv0) {
-		argv0_path = xstrndup(argv0, slash - argv0);
-		return slash + 1;
+		/* ... it's either an absolute path */
+		if (is_absolute_path(argv0)) {
+			/* FIXME: memory leak here */
+			argv0_path = xstrndup(argv0, slash - argv0);
+			return slash + 1;
+		}
+
+		/* ... or a relative path, in which case we have to make it
+		 * absolute first and do the whole thing again */
+		abs_argv0 = xstrdup(real_path(argv0));
+	} else {
+		/* argv0 is no path at all, just a name. Resolve it into a
+		 * path. Unfortunately, this gets system specific. */
+#if defined(__linux__)
+		struct stat st;
+		if (!stat("/proc/self/exe", &st)) {
+			abs_argv0 = xstrdup(real_path("/proc/self/exe"));
+		}
+#elif defined(__APPLE__)
+		/* Mac OS X has realpath, which incidentally allocates its own
+		 * memory, which in turn is why we do all the xstrdup's in the
+		 * other cases. */
+		abs_argv0 = realpath(argv0, NULL);
+#endif
+
+		/* if abs_argv0 is still NULL here, something failed above or
+		 * we are on an unsupported system. system_path() will warn
+		 * and fall back to the static prefix */
+		if (!abs_argv0) {
+			argv0_path = NULL;
+			return argv0;
+		}
 	}
 
-	return argv0;
+	/* abs_argv0 is an absolute path now for which memory was allocated
+	 * with malloc */
+
+	slash = abs_argv0 + strlen(abs_argv0);
+	while (abs_argv0 <= slash && !is_dir_sep(*slash))
+		slash--;
+
+	/* FIXME: memory leaks here */
+	argv0_path = xstrndup(abs_argv0, slash - abs_argv0);
+	slash = xstrdup(slash + 1);
+	free(abs_argv0);
+	return slash;
 }
 
 void git_set_argv_exec_path(const char *exec_path)
-- 
1.7.3.4
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

^ permalink raw reply related

* Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle
From: Junio C Hamano @ 2012-11-27 17:00 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Jeff King, Ilari Liusvaara, Sverre Rabbelier, Elijah Newren,
	Thiago Farina
In-Reply-To: <1353727520-26039-4-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Just shuffle the die() part to make it more explicit, and cleanup the
> code-style.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  transport-helper.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 32ad877..0c95101 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -775,6 +775,9 @@ static int push_refs_with_export(struct transport *transport,
>  		char *private;
>  		unsigned char sha1[20];
>  
> +		if (ref->deletion)
> +			die("remote-helpers do not support ref deletion");
> +
>  		if (!data->refspecs)
>  			continue;

This is not just "just shuffle the die to make it explicit" but it
does change the semantics; earlier ref->deletion was perfectly fine
as long as data->refspecs is not given, but the new code always
dies.

If this semantic change is a good thing, please explain why it is so
in the log message.  If the change is "it does not matter because
when data->refspecs is not given and ref->deletion is set, we die
later elsewhere in the code anyway", then it needs to be described.


Thanks.

> @@ -784,10 +787,6 @@ static int push_refs_with_export(struct transport *transport,
>  		}
>  		free(private);
>  
> -		if (ref->deletion) {
> -			die("remote-helpers do not support ref deletion");
> -		}
> -
>  		if (ref->peer_ref)
>  			string_list_append(&revlist_args, ref->peer_ref->name);

^ permalink raw reply

* Re: [PATCH] Documentation: improve phrasing in git-push.txt
From: Junio C Hamano @ 2012-11-27 17:09 UTC (permalink / raw)
  To: Mark Szepieniec; +Cc: git
In-Reply-To: <1353980254-8033-1-git-send-email-mszepien@gmail.com>

Sounds better; thanks.

^ permalink raw reply

* Re: [PATCH 7/7] push: clarify rejection of update to non-commit-ish
From: Junio C Hamano @ 2012-11-27 17:11 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <CAEUsAPZq1POKQZd5vZy55nEj2kB4CqgxL9_d_0sQs33P_Gcemg@mail.gmail.com>

Chris Rorvick <chris@rorvick.com> writes:

>> With this code, the old must be a commit but new can be a tag that
>> points at a commit?  Why?
>
> The old must not be a tag because fast-forwarding from it is
> potentially destructive; a tag would likely be left dangling in this
> case.  This is not true for the new, though.   I'm not sure
> fast-forwarding from a commit to a tag is useful, but I didn't see a
> reason to prevent it either.   The refs/tags/ hierarchy is excluded
> from fast-forwarding before this check, and refs/heads/ is already
> protected against anything but commits.  So it seems very unlikely
> that someone would accidentally make use of this behavior.
>
> So, fast-forwarding to a tag seemed fairly benign and unlikely to
> cause confusion, so I leaned towards allowing it in case someone found
> a use case for it.

Sounds sensible.

Perhaps some of that thinking behind this change (i.e. earlier
we checked the forwardee was any commit-ish, but the new code only
allows a commit object if it were to be fast-forwarded) belongs to
the log message?

Thanks.

^ permalink raw reply

* Re: [PATCH 5/7] push: require force for refs under refs/tags/
From: Junio C Hamano @ 2012-11-27 17:12 UTC (permalink / raw)
  To: Chris Rorvick
  Cc: git, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras
In-Reply-To: <CAEUsAPZTycJS_USj-tYNN2Bpwn8XvYDTd4c7wFMFDYfNeSCUtw@mail.gmail.com>

Chris Rorvick <chris@rorvick.com> writes:

> On Mon, Nov 26, 2012 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Chris Rorvick <chris@rorvick.com> writes:
>>
>>> diff --git a/remote.c b/remote.c
>>> index 4a6f822..012b52f 100644
>>> --- a/remote.c
>>> +++ b/remote.c
>>> @@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>>>                *
>>>                * (1) if the old thing does not exist, it is OK.
>>>                *
>>> -              * (2) if you do not have the old thing, you are not allowed
>>> +              * (2) if the destination is under refs/tags/ you are
>>> +              *     not allowed to overwrite it; tags are expected
>>> +              *     to be static once created
>>> +              *
>>> +              * (3) if you do not have the old thing, you are not allowed
>>>                *     to overwrite it; you would not know what you are losing
>>>                *     otherwise.
>>>                *
>>> -              * (3) if both new and old are commit-ish, and new is a
>>> +              * (4) if both new and old are commit-ish, and new is a
>>>                *     descendant of old, it is OK.
>>>                *
>>> -              * (4) regardless of all of the above, removing :B is
>>> +              * (5) regardless of all of the above, removing :B is
>>>                *     always allowed.
>>>                */
>>
>> We may want to reword (0) to make it clear that --force
>> (and +A:B) can be used to defeat all the other rules.
>
> On that note, having (5) be "regardless of all of the above ..." seems
> a little awkward.  That would seem to fit better towards the top.

Sure.  (0) you can always force; (1) you can always delete; and then
other requirements.  That may indeed read better.

Thanks.

^ permalink raw reply

* Re: [PATCH] Support for git aliasing for tcsh completion
From: Junio C Hamano @ 2012-11-27 17:16 UTC (permalink / raw)
  To: Marc Khouzam; +Cc: git, Felipe Contreras, SZEDER Gábor, Tuncer Ayaz
In-Reply-To: <CAFj1UpE5V5fKtt0fFOXLPrsQdOL8xpvzT=66Qi3=cMHit092Rg@mail.gmail.com>

The patch was linewrapped so I had to fix it up; please double check
what will be queued on 'pu' to make sure that I did not miss
necessary whitespaces or added unnecessary ones when I rejoined long
lines.

Thanks.

^ permalink raw reply

* Operations on unborn branch
From: Martin von Zweigbergk @ 2012-11-27 17:25 UTC (permalink / raw)
  To: git

While looking at how to handle "git rebase --root", I noticed that
"git cherry-pick" fails with the following when run on an unborn
branch:

error: You do not have a valid HEAD
fatal: cherry-pick failed

I can not see any reason that it shouldn't work. "git cherry-pick -n"
does work. (For rebase, "git cherry-pick --ff" would be used, and I
think that should also work on an unborn branch.)

Also, "git reset" doesn't work on an unborn branch and I can not see
any reason that it shouldn't work. This was also asked on stack
overflow [1], and of course the solution is to use "git rm --cached",
but doesn't mean that "git reset" shouldn't work.

I have very limited time to work on git these days, so if anyone else
would like to work on any of this, I would be very happy. I _might_
take some time to fix the cherry-pick issue.

Btw, every time I run into problems like these with the treatment of
root commits, I can't help but wonder how things would look if git had
always had a single root commit (naturally with some dummy user,
timestamp etc to ensure sameness across repos). With my limited
knowledge, it seems like that would complicate a few things, but
simplify a lot of things (maybe I'm biased because of the things I
have happened to work on?). Has anyone spent some time seriously
thinking about this? I suppose it would be hard to introduce
backward-compatibly, and maybe this is very unrealistic even for git
2.0, but I would be curious to hear what others think.

Martin

[1] http://stackoverflow.com/questions/3894808/new-git-repository-and-already-git-reset-does-not-work

^ permalink raw reply

* Re: Interesting git-format-patch bug
From: Junio C Hamano @ 2012-11-27 17:29 UTC (permalink / raw)
  To: Perry Hutchison; +Cc: git, alan.r.olsen
In-Reply-To: <50b4304c.EwQy4JquPwsUyMfZ%perryh@pluto.rain.com>

perryh@pluto.rain.com (Perry Hutchison) writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>> "Olsen, Alan R" <alan.r.olsen@intel.com> writes:
>> > I found an interesting bug in git-format-patch.
>> >
>> > Say you have a branch A.  You create branch B and add a patch to
>> > it. You then merge that patch into branch A. After the merge,
>> > some other process (we will call it 'gerrit') uses annotate and
>> > changes the comment on the patch that exists on branch B.
>> >
>> > Now someone runs git-format-patch for the last n patches on
>> > branch A.  You should just get the original patch that was
>> > merged over to branch A.  What you get is the patch that was
>> > merged to branch A *and* the patch with the modified commit
>> > comment on branch B. (Double the patches, double the
>> > clean-up...)
>>
>> As you literally have patches that do essentially the same or
>> similar things on two branches that was merged, you cannot
>> expect to export each individual commit into a patch and not
>> have conflicts among them.  So I do not think there is no
>> answer than "don't do that".
>
> To me, this seems to miss Alan's point:  only one patch was merged
> to branch A,...

Are you sure about this part?

I thought Alan's description was that he originally had this

    x-----A
     \     \
      B-----M (a)

and then "some other process" made it like so:

    x-----A
    |\     \
    | B-----M
     \       \
      B'------M' (a)

and then you ask to linealize the last n patches starting from the
rewritten M'.

If that "some other process" instead created a history like this:

    x-----A---\
    |\     \   \ 
    | B-----M   \
     \           \
      B'----------M' (a)

then the redone-merge M' will not see the old B that was fixed later
to B' in the history, but then format-patch would not show B so we
wouldn't be having this discussion thread.

It is possible that "some other process" may (ab)use the parent
field to record the evolution of B, to create a topology like this:

    x-----A---\
    |\     \   \
    | B-----M   \
     \ \         \
      \-B'--------M' (a)

in which case M' has parent B' but B' has a (phoney) parent B.

So again, it all depends on what "some other process" does to the
history when it rewrites it, and if somebody wants to fiter cruft in
the resulting history when flattening it, the knowledge of what
"some other process" does need to help that process.

Which is what I already said, I guess ;-)

> so git-format-patch applied to branch A should find
> only one patch.  It can be argued either way whether that one-patch
> report should include the gerrit annotations, but surely the
> application of gerrit on branch B, _after the merge to branch A
> has already been performed_, should not cause an additional patch
> to magically appear on branch A.

^ permalink raw reply

* Re: [PATCH 1/4] t4041 (diff-submodule-option): parse digests sensibly
From: Junio C Hamano @ 2012-11-27 17:38 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1354005692-2809-2-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> `git rev-list --max-count=1 HEAD` is a roundabout way of saying `git
> rev-parse --verify HEAD`; replace a bunch of instances of the former
> with the latter.  Also, don't unnecessarily `cut -c1-7` the rev-parse
> output when the `--short` option is available.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t4041-diff-submodule-option.sh |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 5377639..cfb71e5 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -21,7 +21,7 @@ add_file () {
>  		test_tick &&
>  		git commit -m "Add $name"
>  	done >/dev/null
> -	git rev-parse --verify HEAD | cut -c1-7
> +	git rev-parse --short --verify HEAD
>  	cd "$owd"
>  }
>  commit_file () {
> @@ -33,7 +33,7 @@ test_create_repo sm1 &&
>  add_file . foo >/dev/null
>  
>  head1=$(add_file sm1 foo1 foo2)
> -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
> +fullhead1=$(cd sm1; git rev-parse --verify $head1)

That still is a roundabout way to say "git rev-parse --verify HEAD",
no?  Why feed a shortened one to get the expanded result when you
know the full representation of HEAD is what you want?

>  test_expect_success 'added submodule' "
>  	git add sm1 &&
> @@ -116,8 +116,8 @@ EOF
>  	test_cmp expected actual
>  "
>  
> -fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
>  test_expect_success 'modified submodule(forward) --submodule=short' "
> +fullhead2=$(cd sm1; git rev-parse --verify $head2)
>  	git diff --submodule=short >actual &&
>  	cat >expected <<-EOF &&
>  diff --git a/sm1 b/sm1
> @@ -135,7 +135,7 @@ commit_file sm1 &&
>  head3=$(
>  	cd sm1 &&
>  	git reset --hard HEAD~2 >/dev/null &&
> -	git rev-parse --verify HEAD | cut -c1-7
> +	git rev-parse --short --verify HEAD
>  )
>  
>  test_expect_success 'modified submodule(backward)' "
> @@ -220,8 +220,8 @@ EOF
>  rm -f sm1 &&
>  test_create_repo sm1 &&
>  head6=$(add_file sm1 foo6 foo7)
> -fullhead6=$(cd sm1; git rev-list --max-count=1 $head6)
>  test_expect_success 'nonexistent commit' "
> +fullhead6=$(cd sm1; git rev-parse --verify $head6)
>  	git diff-index -p --submodule=log HEAD >actual &&
>  	cat >expected <<-EOF &&
>  Submodule sm1 $head4...$head6 (commits not present)
> @@ -318,8 +318,8 @@ EOF
>  "
>  
>  (cd sm1; git commit -mchange foo6 >/dev/null) &&
> -head8=$(cd sm1; git rev-parse --verify HEAD | cut -c1-7) &&
>  test_expect_success 'submodule is modified' "
> +head8=$(cd sm1; git rev-parse --short --verify HEAD) &&
>  	git diff-index -p --submodule=log HEAD >actual &&
>  	cat >expected <<-EOF &&
>  Submodule sm1 $head6..$head8:
> @@ -461,7 +461,7 @@ EOF
>  	test_cmp expected actual
>  "
>  
> -fullhead7=$(cd sm2; git rev-list --max-count=1 $head7)
> +fullhead7=$(cd sm2; git rev-parse --verify $head7)
>  
>  test_expect_success 'given commit --submodule=short' "
>  	git diff-index -p --submodule=short HEAD^ >actual &&

^ permalink raw reply

* [PATCH 4/6] Rearrange the description of remote helper capabilities
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

This also remove some duplication in the descriptions
(e.g. refspec was explained twice with similar level of detail)

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 134 +++++++++++++++--------------------
 1 file changed, 56 insertions(+), 78 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 7eb43d7..7ac1461 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -88,81 +88,17 @@ Each remote helper is expected to support only a subset of commands.
 The operations a helper supports are declared to git in the response
 to the `capabilities` command (see COMMANDS, below).
 
-'option'::
-	For specifying settings like `verbosity` (how much output to
-	write to stderr) and `depth` (how much history is wanted in the
-	case of a shallow clone) that affect how other commands are
-	carried out.
-
-'connect'::
-	For fetching and pushing using git's native packfile protocol
-	that requires a bidirectional, full-duplex connection.
-
-'push'::
-	For listing remote refs and pushing specified objects from the
-	local object store to remote refs.
-
-'fetch'::
-	For listing remote refs and fetching the associated history to
-	the local object store.
-
-'export'::
-	For listing remote refs and pushing specified objects from a
-	fast-import stream to remote refs.
-
-'import'::
-	For listing remote refs and fetching the associated history as
-	a fast-import stream.
-
-'refspec' <refspec>::
-	This modifies the 'import' capability, allowing the produced
-	fast-import stream to modify refs in a private namespace
-	instead of writing to refs/heads or refs/remotes directly.
-	It is recommended that all importers providing the 'import'
-	capability use this.
-+
-A helper advertising the capability
-`refspec refs/heads/*:refs/svn/origin/branches/*`
-is saying that, when it is asked to `import refs/heads/topic`, the
-stream it outputs will update the `refs/svn/origin/branches/topic`
-ref.
-+
-This capability can be advertised multiple times.  The first
-applicable refspec takes precedence.  The left-hand of refspecs
-advertised with this capability must cover all refs reported by
-the list command.  If no 'refspec' capability is advertised,
-there is an implied `refspec *:*`.
-
-'bidi-import'::
-	The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
-	to retrieve information about blobs and trees that already exist in
-	fast-import's memory. This requires a channel from fast-import to the
-	remote-helper.
-	If it is advertised in addition to "import", git establishes a pipe from
-	fast-import to the remote-helper's stdin.
-	It follows that git and fast-import are both connected to the
-	remote-helper's stdin. Because git can send multiple commands to
-	the remote-helper it is required that helpers that use 'bidi-import'
-	buffer all 'import' commands of a batch before sending data to fast-import.
-	This is to prevent mixing commands and fast-import responses on the
-	helper's stdin.
-
-'export-marks' <file>::
-	This modifies the 'export' capability, instructing git to dump the
-	internal marks table to <file> when complete. For details,
-	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
-
-'import-marks' <file>::
-	This modifies the 'export' capability, instructing git to load the
-	marks specified in <file> before processing any input. For details,
-	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+In the following, we list all defined capabilities and for
+each we list which commands a helper with that capability
+must provide.
 
 Capabilities for Pushing
-~~~~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^^^^
 'connect'::
 	Can attempt to connect to 'git receive-pack' (for pushing),
-	'git upload-pack', etc for communication using the
-	packfile protocol.
+	'git upload-pack', etc for communication using
+	git's native packfile protocol. This
+	requires a bidirectional, full-duplex connection.
 +
 Supported commands: 'connect'.
 
@@ -186,11 +122,12 @@ Other frontends may have some other order of preference.
 
 
 Capabilities for Fetching
-~~~~~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^^^^^
 'connect'::
 	Can try to connect to 'git upload-pack' (for fetching),
 	'git receive-pack', etc for communication using the
-	packfile protocol.
+	git's native packfile protocol. This
+	requires a bidirectional, full-duplex connection.
 +
 Supported commands: 'connect'.
 
@@ -212,14 +149,27 @@ connecting (see the 'connect' command under COMMANDS).
 When choosing between 'fetch' and 'import', git prefers 'fetch'.
 Other frontends may have some other order of preference.
 
+Miscellaneous capabilities
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+'option'::
+	For specifying settings like `verbosity` (how much output to
+	write to stderr) and `depth` (how much history is wanted in the
+	case of a shallow clone) that affect how other commands are
+	carried out.
+
 'refspec' <refspec>::
-	This modifies the 'import' capability.
+	This modifies the 'import' capability, allowing the produced
+	fast-import stream to modify refs in a private namespace
+	instead of writing to refs/heads or refs/remotes directly.
+	It is recommended that all importers providing the 'import'
+	capability use this.
 +
-A helper advertising
+A helper advertising the capability
 `refspec refs/heads/*:refs/svn/origin/branches/*`
-in its capabilities is saying that, when it handles
-`import refs/heads/topic`, the stream it outputs will update the
-`refs/svn/origin/branches/topic` ref.
+is saying that, when it is asked to `import refs/heads/topic`, the
+stream it outputs will update the `refs/svn/origin/branches/topic`
+ref.
 +
 This capability can be advertised multiple times.  The first
 applicable refspec takes precedence.  The left-hand of refspecs
@@ -227,6 +177,34 @@ advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
 
+'bidi-import'::
+	This modifies the 'import' capability.
+	The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
+	to retrieve information about blobs and trees that already exist in
+	fast-import's memory. This requires a channel from fast-import to the
+	remote-helper.
+	If it is advertised in addition to "import", git establishes a pipe from
+	fast-import to the remote-helper's stdin.
+	It follows that git and fast-import are both connected to the
+	remote-helper's stdin. Because git can send multiple commands to
+	the remote-helper it is required that helpers that use 'bidi-import'
+	buffer all 'import' commands of a batch before sending data to fast-import.
+	This is to prevent mixing commands and fast-import responses on the
+	helper's stdin.
+
+'export-marks' <file>::
+	This modifies the 'export' capability, instructing git to dump the
+	internal marks table to <file> when complete. For details,
+	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
+
+'import-marks' <file>::
+	This modifies the 'export' capability, instructing git to load the
+	marks specified in <file> before processing any input. For details,
+	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+
+
+
+
 COMMANDS
 --------
 
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 2/6] Document missing remote helper capabilities
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

The 'export' and '(im|ex)port-marks' capabilities were not
documented at all

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 45 +++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 9a7e583..db63541 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -106,6 +106,10 @@ to the `capabilities` command (see COMMANDS, below).
 	For listing remote refs and fetching the associated history to
 	the local object store.
 
+'export'::
+	For listing remote refs and pushing specified objects from a
+	fast-import stream to remote refs.
+
 'import'::
 	For listing remote refs and fetching the associated history as
 	a fast-import stream.
@@ -143,6 +147,16 @@ there is an implied `refspec *:*`.
 	This is to prevent mixing commands and fast-import responses on the
 	helper's stdin.
 
+'export-marks' <file>::
+	This modifies the 'export' capability, instructing git to dump the
+	internal marks table to <file> when complete. For details,
+	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
+
+'import-marks' <file>::
+	This modifies the 'export' capability, instructing git to load the
+	marks specified in <file> before processing any input. For details,
+	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+
 Capabilities for Pushing
 ~~~~~~~~~~~~~~~~~~~~~~~~
 'connect'::
@@ -158,9 +172,18 @@ Supported commands: 'connect'.
 +
 Supported commands: 'list for-push', 'push'.
 
-If a helper advertises both 'connect' and 'push', git will use
-'connect' if possible and fall back to 'push' if the helper requests
-so when connecting (see the 'connect' command under COMMANDS).
+'export'::
+	Can discover remote refs and push specified objects from a
+	fast-import stream to remote refs.
++
+Supported commands: 'list for-push', 'export'.
+
+If a helper advertises 'connect', git will use it if possible and
+fall back to another capability if the helper requests so when
+connecting (see the 'connect' command under COMMANDS).
+When choosing between 'push' and 'export', git prefers 'push'.
+Other frontends may have some other order of preference.
+
 
 Capabilities for Fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -307,6 +330,22 @@ stdin.
 +
 Supported if the helper has the 'import' capability.
 
+'export'::
+	Instructs the remote helper that any subsequent input is
+	part of a fast-import stream (generated by 'git fast-export')
+	containing objects which should be pushed to the remote.
++
+Especially useful for interoperability with a foreign versioning
+system.
++
+The 'export-marks' and 'import-marks' capabilities, if specified,
+affect this command in so far as they are passed on to 'git
+fast-export', which then will load/store a table of marks for
+local objects. This can be used to implement for incremental
+operations.
++
+Supported if the helper has the 'export' capability.
+
 'connect' <service>::
 	Connects to given service. Standard input and standard output
 	of helper are connected to specified service (git prefix is
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 3/6] Fix grammar
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>


Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index db63541..7eb43d7 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
 'capabilities'::
 	Lists the capabilities of the helper, one per line, ending
 	with a blank line. Each capability may be preceded with '*',
-	which marks them mandatory for git version using the remote
-	helper to understand (unknown mandatory capability is fatal
-	error).
+	which marks them mandatory for git versions using the remote
+	helper to understand. Any unknown mandatory capability is a
+	fatal error.
 
 'list'::
 	Lists the refs, one per line, in the format "<value> <name>
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 5/6] Make clearer which commands must be supported for which capabilities
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

In particular, document 'list for-push' separately from 'list',
as the former needs only be supported for the 'push' capability,
and the latter only for fetch/import/export. In particular,
a hypothetically 'push-only' helper only needs to support the
former, not the latter.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 7ac1461..023dcca 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -216,6 +216,8 @@ Commands are given by the caller on the helper's standard input, one per line.
 	which marks them mandatory for git versions using the remote
 	helper to understand. Any unknown mandatory capability is a
 	fatal error.
++
+Support for this command is mandatory.
 
 'list'::
 	Lists the refs, one per line, in the format "<value> <name>
@@ -225,9 +227,18 @@ Commands are given by the caller on the helper's standard input, one per line.
 	the name; unrecognized attributes are ignored. The list ends
 	with a blank line.
 +
-If 'push' is supported this may be called as 'list for-push'
-to obtain the current refs prior to sending one or more 'push'
-commands to the helper.
+Supported if the helper has the "fetch" or "import" capability.
+
+'list for-push'::
+	Similar to 'list', except that it is used if and only if
+	the caller wants to the resulting ref list to prepare
+	push commands.
+	A helper supporting both push and fetch can use this
+	to distinguish for which operation the output of 'list'
+	is going to be used, possibly reducing the amount
+	of work that needs to be performed.
++
+Supported if the helper has the "push" or "export" capability.
 
 'option' <name> <value>::
 	Sets the transport helper option <name> to <value>.  Outputs a
@@ -306,7 +317,7 @@ sequence has to be buffered before starting to send data to fast-import
 to prevent mixing of commands and fast-import responses on the helper's
 stdin.
 +
-Supported if the helper has the 'import' capability.
+Supported if the helper has the "import" capability.
 
 'export'::
 	Instructs the remote helper that any subsequent input is
@@ -322,7 +333,7 @@ fast-export', which then will load/store a table of marks for
 local objects. This can be used to implement for incremental
 operations.
 +
-Supported if the helper has the 'export' capability.
+Supported if the helper has the "export" capability.
 
 'connect' <service>::
 	Connects to given service. Standard input and standard output
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 6/6] Remove 'for-push' from ref list attributes list, link to subsections
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

The documentation was misleading in that it gave the impression that
'for-push' could be used as a ref attribute in the output of the
'list' command. That is wrong.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 023dcca..081cb06 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -227,6 +227,8 @@ Support for this command is mandatory.
 	the name; unrecognized attributes are ignored. The list ends
 	with a blank line.
 +
+See REF LIST ATTRIBUTES for a list of currently defined options.
++
 Supported if the helper has the "fetch" or "import" capability.
 
 'list for-push'::
@@ -248,6 +250,8 @@ Supported if the helper has the "push" or "export" capability.
 	for it).  Options should be set before other commands,
 	and may influence the behavior of those commands.
 +
+See OPTIONS for a list of currently defined options.
++
 Supported if the helper has the "option" capability.
 
 'fetch' <sha1> <name>::
@@ -256,7 +260,7 @@ Supported if the helper has the "option" capability.
 	per line, terminated with a blank line.
 	Outputs a single blank line when all fetch commands in the
 	same batch are complete. Only objects which were reported
-	in the ref list with a sha1 may be fetched this way.
+	in the output of 'list' with a sha1 may be fetched this way.
 +
 Optionally may output a 'lock <file>' line indicating a file under
 GIT_DIR/objects/pack which is keeping a pack until refs can be
@@ -360,10 +364,9 @@ capabilities reported by the helper.
 REF LIST ATTRIBUTES
 -------------------
 
-'for-push'::
-	The caller wants to use the ref list to prepare push
-	commands.  A helper might chose to acquire the ref list by
-	opening a different type of connection to the destination.
+The 'list' command produces a list of refs in which each ref
+may be followed by a list of attributes. The following ref list
+attributes are defined.
 
 'unchanged'::
 	This ref is unchanged since the last import or fetch, although
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 1/6] Document invocation first, then input format
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn
In-Reply-To: <1354038279-76475-1-git-send-email-max@quendi.de>

In the distant past, the order was 'Invocation', 'Commands',
'Capabilities', ...

Then it was decided that before giving a list of Commands, there
should be an overall description of the 'Input format', which was
a wise decision. However, this description was put as the very
first thing, with the rationale that any implementor would want
to know that first.

However, it seems an implementor would actually first need to
know how the remote helper will be invoked, so moving
'Invocation' to the front again seems logical. Moreover, we now
don't switch from discussing the input format to the invocation
style and then back to input related stuff.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 62 ++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 5ce4cda..9a7e583 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -35,6 +35,37 @@ transport protocols, such as 'git-remote-http', 'git-remote-https',
 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
 'fetch', 'option', and 'push'.
 
+INVOCATION
+----------
+
+Remote helper programs are invoked with one or (optionally) two
+arguments. The first argument specifies a remote repository as in git;
+it is either the name of a configured remote or a URL. The second
+argument specifies a URL; it is usually of the form
+'<transport>://<address>', but any arbitrary string is possible.
+The 'GIT_DIR' environment variable is set up for the remote helper
+and can be used to determine where to store additional data or from
+which directory to invoke auxiliary git commands.
+
+When git encounters a URL of the form '<transport>://<address>', where
+'<transport>' is a protocol that it cannot handle natively, it
+automatically invokes 'git remote-<transport>' with the full URL as
+the second argument. If such a URL is encountered directly on the
+command line, the first argument is the same as the second, and if it
+is encountered in a configured remote, the first argument is the name
+of that remote.
+
+A URL of the form '<transport>::<address>' explicitly instructs git to
+invoke 'git remote-<transport>' with '<address>' as the second
+argument. If such a URL is encountered directly on the command line,
+the first argument is '<address>', and if it is encountered in a
+configured remote, the first argument is the name of that remote.
+
+Additionally, when a configured remote has 'remote.<name>.vcs' set to
+'<transport>', git explicitly invokes 'git remote-<transport>' with
+'<name>' as the first argument. If set, the second argument is
+'remote.<name>.url'; otherwise, the second argument is omitted.
+
 INPUT FORMAT
 ------------
 
@@ -173,37 +204,6 @@ advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
 
-INVOCATION
-----------
-
-Remote helper programs are invoked with one or (optionally) two
-arguments. The first argument specifies a remote repository as in git;
-it is either the name of a configured remote or a URL. The second
-argument specifies a URL; it is usually of the form
-'<transport>://<address>', but any arbitrary string is possible.
-The 'GIT_DIR' environment variable is set up for the remote helper
-and can be used to determine where to store additional data or from
-which directory to invoke auxiliary git commands.
-
-When git encounters a URL of the form '<transport>://<address>', where
-'<transport>' is a protocol that it cannot handle natively, it
-automatically invokes 'git remote-<transport>' with the full URL as
-the second argument. If such a URL is encountered directly on the
-command line, the first argument is the same as the second, and if it
-is encountered in a configured remote, the first argument is the name
-of that remote.
-
-A URL of the form '<transport>::<address>' explicitly instructs git to
-invoke 'git remote-<transport>' with '<address>' as the second
-argument. If such a URL is encountered directly on the command line,
-the first argument is '<address>', and if it is encountered in a
-configured remote, the first argument is the name of that remote.
-
-Additionally, when a configured remote has 'remote.<name>.vcs' set to
-'<transport>', git explicitly invokes 'git remote-<transport>' with
-'<name>' as the first argument. If set, the second argument is
-'remote.<name>.url'; otherwise, the second argument is omitted.
-
 COMMANDS
 --------
 
-- 
1.8.0.393.gcc9701d

^ permalink raw reply related

* [PATCH 0/6] Improve remote helper documentation
From: Max Horn @ 2012-11-27 17:44 UTC (permalink / raw)
  To: git; +Cc: Max Horn

Various remote helper capabilities and commands were not
documented, in particular 'export', or documented in a misleading
way (e.g. 'for-push' was listed as a ref attribute understood by
git, which is not the case). This patch series changes that, and
also address some other things in the remote helper documentation
that I found jarring when reading through it.

Note that the description of export and (im|ex)port-marks probably can be
improved, and I hope that somebody who knows more about them
than me and/or is better at writing documentation will do just that.
But I felt it was better to provide something than to do nothing
and only complain, as I did previously on this subject ;-).

Max Horn (6):
  Document invocation first, then input format
  Document missing remote helper capabilities
  Fix grammar
  Rearrange the description of remote helper capabilities
  Make clearer which commands must be supported for which capabilities
  Remove 'for-push' from ref list attributes list, link to subsections

 Documentation/git-remote-helpers.txt | 241 ++++++++++++++++++++---------------
 1 file changed, 136 insertions(+), 105 deletions(-)

-- 
1.8.0.393.gcc9701d

^ permalink raw reply

* Why is this known by git but not by gitk
From: Kevin O'Gorman @ 2012-11-27 17:50 UTC (permalink / raw)
  To: git

I just converted a SourceForge CVS repo to git using cvs2svn.  One
directory in the result is named CVSROOT and everything would be fine,
but I wan to know why gitk does not see it.

Git itself does not report it either, but it lets me "git rm -r
CVSROOT" and stages the change.

I just want to know what's up with this before I commit myself to
using this converted repo.

--
Kevin O'Gorman

programmer, n. an organism that transmutes caffeine into software.

^ permalink raw reply

* Re: [PATCH 4/4] t4041 (diff-submodule-option): change tense of test names
From: Junio C Hamano @ 2012-11-27 17:51 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1354005692-2809-5-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Change the tense of test names from past to present, as this is the
> prevalent style.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---

I see most of them are not "past" but "past particle" used as if
they are adjectives.

For example, I think this test

> -test_expect_success 'added submodule' '

tries to say "See what happens to an added submodule".

The same for the others.

> -test_expect_success 'modified submodule(forward)' '

"See what is shown for modified submodule."

> -test_expect_success 'typechanged submodule(submodule->blob), --cached' '

"See what is shown for typechanged one, when --cached option is
given".

So I do not think this patch is needed; the current wording looks
not so grammatically kosher, but still is understandable.  Updated
result isn't.

^ permalink raw reply

* Re: [PATCH] Third try at documenting command integration requirements.
From: Eric S. Raymond @ 2012-11-27 17:56 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <50B4A8E1.7050801@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu>:
> OK, now let's discuss *which* minimum Python version that git should
> support in the hypothetical new world...

By all means!
 
> It would be a shame to leave RHEL 5 users behind if Python is used to
> implement important git functionality.  Python 2.4 is missing some of
> Python's shiny new features, but still quite OK.  What features would
> you miss the most if we were to target Python 2.4 instead of 2.6?

Off the top of my head...the 'with' statement, the conditional
expression, and built-in JSON support.  Other developers would be
likely to kick about the string format() method; personally I'm
cheerfully old-school about that.

I agree that 2.4 is still quite OK.  I'm a little concerned that dropping that
far back might store up some transition problems for the day we decide to
make the jump to Python 3.

On the other hand, I think gating features on RHEL5 might be
excessively cautious.  According to [1], RHEL will red-zone within 30
days if it hasn't done so already ([1] says "Q4").  And RHEL6 (with
Python 2.6) has been shipping for two years.

Policy suggestion: we aim to stay friendly for every version of RHEL that
is still in Support 1.  I doubt anyone will code anything critical 
in Python before Dec 31st - I'm certainly not planning to!

[1] http://en.wikipedia.org/wiki/Red_Hat_Enterprise_Linux RHEL5 is going
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: [PATCH 2/4] t4041 (diff-submodule-option): rewrite add_file() routine
From: Junio C Hamano @ 2012-11-27 17:56 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1354005692-2809-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Instead of "cd there and then come back", use the "cd there in a
> subshell" pattern.  Also fix '&&' chaining in one place.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t4041-diff-submodule-option.sh |   13 +++++--------
>  1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index cfb71e5..103c690 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -11,18 +11,15 @@ This test tries to verify the sanity of the --submodule option of git diff.
>  . ./test-lib.sh
>  
>  add_file () {
> +	(cd "$1" &&
> +	    shift &&
> +	    for name; do
>  		echo "$name" > "$name" &&
>  		git add "$name" &&
>  		test_tick &&
>  		git commit -m "Add $name"
> +	    done >/dev/null &&
> +	    git rev-parse --short --verify HEAD)

While at it, why not do the "indent with a single tab", and other
style fixes?  e.g.

	(
		cd "$1" &&
                shift &&
                for name
                do
			echo "$name" >"$name" &&
                        git add $name" &&
                        test_tick &&
                        git commit -m "Add $name" || exit
		done >/dev/null &&
		git rev-parse --short --verify HEAD
	)

The "|| exit" is needed to catch failures inside the loop (not that
"git commit" there is likely to fail, so it is just for
completeness).

^ permalink raw reply

* Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: Heiko Voigt @ 2012-11-27 18:31 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <cover.1353962698.git.wking@tremily.us>

Hi,

On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote:
> From: "W. Trevor King" <wking@tremily.us>
> 
> On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote:
> > We could add
> >
> >   $ git submodule update --branch
> >
> > to checkout the gitlinked SHA1 as submodule.<name>.branch in each of
> > the submodules, leaving the submodules on the .gitmodules-configured
> > branch.  Effectively (for each submodule):
> >
> >   $ git branch -f $branch $sha1
> >   $ git checkout $branch
> 
> I haven't gotten any feedback on this as an idea, but perhaps someone
> will comment on it as a patch series ;).

I am not sure I understand you correctly. You are suggesting that the
branch option as an alias for the registered SHA1 in the superproject?

I though the goal of your series was that you want to track submodules
branch which come from the remote side?

Doing the above does not assist you much in that does it?

I would think more of some convention like:

	$ git checkout -t origin/$branch

when first initialising the submodule with e.g.

	$ git submodule update --init --branch

Then later calls of

	$ git submodule update --branch

would have a branch configured to pull from. I imagine that results in
a similar behavior gerrit is doing on the server side?

> Changes since v3:
> 
> * --record=??? is now --local-branch=???
> * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation)
> * Added local git-config overrides of .gitmodules' submodule.<name>.branch
> * Added `submodule update --branch`

I would prefer if we could squash all these commits together into one
since it seems to me one logical step, using the new variable for update
belongs together with its configuration on initialization.

How about reusing the -b|--branch option for add? Since we only change
the behavior when submodule.$name.update is set to branch it seems
reasonable to me. Opinions?

> Because you need to recurse through submodules for `update --branch`
> even if "$subsha1" == "$sha1", I had to amend the conditional
> controlling that block.  This broke one of the existing tests, which I
> "fixed" in patch 4.  I think a proper fix would involve rewriting
> 
>   (clear_local_git_env; cd "$sm_path" &&
>    ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
>     test -z "$rev") || git-fetch)) ||
>   die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> 
> but I'm not familiar enough with rev-list to want to dig into that
> yet.  If feedback for the earlier three patches is positive, I'll work
> up a clean fix and resubmit.

You probably need to separate your handling here. The comparison of the
currently checked out sha1 and the recorded sha1 is an optimization
which skips unnecessary fetching in case the submodules commits are
already correct. This code snippet checks whether the to be checked out
sha1 is already local and also skips the fetch if it is. We should not
break that.

Maybe we need an else block here and possibly extract the current code
inside the if statement into a function. E.g. that the final code looks
something like this:

	if test "$subsha1" != "$sha1"
	then
		handle_on_demand_fetch_update ...
	else
		handle_tracked_branch_update ...
	fi

Not sure about the function names though. If we decide to go that route:
The extraction into a function should go in an extra preparation patch
which does not change any functionality.

I will reply to the patches for further comments.

Cheers Heiko

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-27 18:35 UTC (permalink / raw)
  To: Magnus Bäck; +Cc: Felipe Contreras, Michael Haggerty, git
In-Reply-To: <20121127143510.GA15831@google.com>

Magnus Bäck <baeck@google.com>:
> While "constant traffic" probably overstates the issue, these are not
> theoretical problems. I recall at least three cases in the last year
> or so where Git has seen breakage with Solaris or Mac OS X because
> of sed or tr incompatibilities, and I don't even read this list that
> thoroughly.

This is exactly the sort of of pain experience would lead me to
expect.  

OK, this is where I assume the full Old Fart position (30-year
old-school Unix guy, author of "The Art of Unix Programming", can
remember the years before Perl and still has sh idioms in his
fingertips) and say "Get with the 21st century, people! Or at least
1990..."

As a general scripting language shell sucks *really badly* compared to
anything new-school. Performance, portability, you name it, it's a
mess.  It's not so much the shell interpreters itself that are the
portabilty problem, but (as Magnus implicitly points out) all those
userland dependencies on sed and tr and awk and even variants of
expr(!) that get dragged in the second you try to get any actual work
done.

Can we cease behaving like we're still pounding keys on 110-baud
teletypes now?  Some old-school Unix habits have persisted long past
the point that they're even remotely sane.  Shell programming at any
volume above a few lines of throwaway code is one of them - it's
*nuts* and we should *stop doing it*.

(Yes, I too still make this mistake occasionally out of ancient reflex.
But I know I shouldn't, and I always end up regretting it.)
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: [PATCH v4 3/4] git-submodule update: Add --branch option
From: Heiko Voigt @ 2012-11-27 18:51 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <95edff1c97c513c555652014f9c2bbf61c8e7560.1353962698.git.wking@tremily.us>

On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote:
> From: "W. Trevor King" <wking@tremily.us>
> 
> This allows users to checkout the current
> superproject-recorded-submodule-sha as a branch, avoiding the detached
> head state that the standard submodule update creates.  This may be
> useful for the existing --rebase/--merge workflows which already avoid
> detached heads.
> 
> It is also useful if you want easy tracking of upstream branches.  The
> particular upstream branch to be tracked is configured locally with
> .git/modules/<name>/config.  With the new option Ævar's suggested
> 
>   $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitm
> odules submodule.$name.branch) && git pull'
> 
> reduces to a
> 
>   $ git submodule update --branch
> 
> after each supermodule .gitmodules edit, and a
> 
>   $ git submodule foreach 'git pull'
> 
> whenever you feel like updating the submodules.  Your still on you're
> own to commit (or not) the updated submodule hashes in the
> superproject's .gitmodules.
> 
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  Documentation/git-submodule.txt | 20 +++++++++++------
>  git-submodule.sh                | 48 +++++++++++++++++++++++++++++----------
>  t/t7406-submodule-update.sh     | 50 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 98 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index d0b4436..34392a1 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
>  	      [-f|--force] [--reference <repository>] [--] <repository> [<path>]
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
> -'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> +'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--branch] [--rebase]
>  	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
>  	      [commit] [--] [<path>...]
> @@ -136,11 +136,11 @@ init::
>  
>  update::
>  	Update the registered submodules, i.e. clone missing submodules and
> -	checkout the commit specified in the index of the containing repository.
> -	This will make the submodules HEAD be detached unless `--rebase` or
> -	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> -	`--checkout`.
> +	checkout the commit specified in the index of the containing
> +	repository.  This will make the submodules HEAD be detached unless
> +	`--branch`, `--rebase`, `--merge` is specified or the key
> +	`submodule.$name.update` is set to `branch`, `rebase`, `merge` or
> +	`none`. `none` can be overridden by specifying `--checkout`.
>  +
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
> @@ -207,7 +207,13 @@ OPTIONS
>  
>  -b::
>  --branch::
> -	Branch of repository to add as submodule.
> +	When used with the add command, gives the branch of repository to
> +	add as submodule.
> ++
> +When used with the update command, checks out a branch named
> +`submodule.<name>.branch` (as set by `--local-branch`) pointing at the
> +current HEAD SHA-1.  This is useful for commands like `update
> +--rebase` that do not work on detached heads.

Since you are reusing this option for update it further convinces me
that reusing it for add makes sense and simplifies the logic for users.

I think an optional argument for --branch would be nice in the update
case:

	$ git submodule update --branch=master

would then allow a user that has not configured anything (except the
branch tracking info in the submodule of course) to pull all submodules
master branches.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index c51b6ae..28eb4b1 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")"
>  			die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
>  		fi
>  
> -		if test "$subsha1" != "$sha1" -o -n "$force"
> +		if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch"

As said before I think separating your code from the current update
logic will simplify the handling below.

>  		then
>  			subforce=$force
>  			# If we don't already have a -f flag and the submodule has never been checked out
> @@ -650,16 +654,21 @@ Maybe you want to use 'update --init'?")"
>  			case ";$cloned_modules;" in
>  			*";$name;"*)
>  				# then there is no local change to integrate
> -				update_module= ;;
> +				case "$update_module" in
> +					rebase|merge)
> +						update_module=
> +						;;
> +				esac
> +				;;
>  			esac
>  
>  			must_die_on_failure=
>  			case "$update_module" in
>  			rebase)
>  				command="git rebase"
> -				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
> +				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"	
>  				say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
> -				must_die_on_failure=yes
> +			must_die_on_failure=yes

Please always cleanup whitespace changes.

>  				;;
>  			merge)
>  				command="git merge"
> @@ -674,15 +683,30 @@ Maybe you want to use 'update --init'?")"
>  				;;
>  			esac
>  
>  			then
> -				die_with_status 2 "$die_msg"
> -			else
> -				err="${err};$die_msg"
> -				continue
> +				if (clear_local_git_env; cd "$sm_path" &&
> +					git branch -f "$branch" "$sha1" &&
> +					git checkout "$branch")

You wrote in earlier emails that you wanted to protect the user from
non-fastforward changes. So I would expect a

	$ git pull --ff-only

here and the setup of that in the initialization of the submodule.

BTW, I am more and more convinced that an automatically manufactured
commit on update with --branch should be the default. What do other
think? Sascha raised a concern that he would not want this, but as far as
I understood he let the CI-server do that so I see no downside to
natively adding that to git. People who want to manually craft those
commits can still amend the generated commit. Since this is all about
helping people keeping their submodules updated why not go the full way?

Cheers Heiko

^ 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