Git development
 help / color / mirror / Atom feed
* [EGIT PATCH] Use the possible resources's specific encoding for quickdiff
From: Robin Rosenberg @ 2008-12-20 23:14 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg

Encoding can be defined in many places. It could be defined for a specific
resource, workspace, JVM invocation or platform. Let Eclipse handle the
logic. We always ask for the current revisions encoding, which in theory
could be different from the encoding specified for the version we are
retrieving.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../egit/ui/internal/decorators/GitDocument.java   |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index a985a68..a9c0c7d 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -9,8 +9,10 @@
 
 import java.io.IOException;
 
+import org.eclipse.core.resources.IEncodedStorage;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jface.text.Document;
 import org.eclipse.team.core.RepositoryProvider;
 import org.spearce.egit.core.GitProvider;
@@ -66,7 +68,23 @@ void populate() throws IOException {
 			Activator.trace("(GitQuickDiffProvider) compareTo: " + baseline);
 			ObjectLoader loader = repository.openBlob(blobEnry.getId());
 			byte[] bytes = loader.getBytes();
-			String s = new String(bytes); // FIXME Platform default charset. should be Eclipse default
+			String charset;
+			// Get the encoding for the current version. As a matter of
+			// principle one might want to use the eclipse settings for the
+			// version we are retrieving as that may be defined by the
+			// project settings, but there is no historic API for this.
+			IEncodedStorage encodedStorage = ((IEncodedStorage)resource);
+			try {
+				if (encodedStorage != null)
+					charset = encodedStorage.getCharset();
+				else
+					charset = resource.getParent().getDefaultCharset();
+			} catch (CoreException e) {
+				charset = Constants.CHARACTER_ENCODING;
+			}
+			// Finally we could consider validating the content with respect
+			// to the content. We don't do that here.
+			String s = new String(bytes, charset);
 			set(s);
 			Activator.trace("(GitQuickDiffProvider) has reference doc, size=" + s.length() + " bytes");
 		} else {
-- 
1.6.1.rc3.56.gd0306

^ permalink raw reply related

* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-20 23:31 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: git, Junio C Hamano
In-Reply-To: <200812201654.23110.bss@iguanasuicide.net>

lördag 20 december 2008 23:54:19 skrev Boyd Stephen Smith Jr.:
> On Saturday 2008 December 20 01:08:01 Robin Rosenberg wrote:
> > fredag 19 december 2008 03:39:15 skrev Boyd Stephen Smith Jr.:
> > > On Thursday 2008 December 18 18:21:25 Linus Torvalds wrote:
> > > > I suspect we should warn about reverting merges.
> >
> > Or mention the reverted parent in the commit message since it is not
> > obvious.
> >
> > ---
> >  builtin-revert.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/builtin-revert.c b/builtin-revert.c
> > index 4038b41..fc59229 100644
> > --- a/builtin-revert.c
> > +++ b/builtin-revert.c
> > @@ -352,6 +352,10 @@ static int revert_or_cherry_pick(int argc, const char
> > **argv) add_to_msg(oneline_body + 1);
> >  		add_to_msg("\"\n\nThis reverts commit ");
> >  		add_to_msg(sha1_to_hex(commit->object.sha1));
> > +		if (commit->parents->next) {
> > +			add_to_msg(" removing\ncontributions from ");
> > +			add_to_msg(sha1_to_hex(parent->object.sha1));
> > +		}
> >  		add_to_msg(".\n");
> >  	} else {
> >  		base = parent;
> 
> I'm still new to the code, but parent is the "mainline" specified on the 
> command-line, which (I think) is actually the parent to be reverted to, so we 
> are actually removing contributions from all the *other* parents.  So, the 
> message may be backward.  Because of that, I'd say the patch doesn't handle 

Indeed the message is backward. How about  "removing all contributions except from"... etc ?

An alternative, would be "removing changes relative to .." (mainline). The changes are
the contributions from all other parents. I have to huge interest in the exact phrase used.

> octopus merges well, either.

Same problem, I think.

-- robin

^ permalink raw reply

* Re: how to check remote git repo for updates without pull/fetch
From: David Aguilar @ 2008-12-20 23:41 UTC (permalink / raw)
  To: James Cloos; +Cc: Ivan Zorin, git
In-Reply-To: <m3skoi21m3.fsf@lugabout.jhcloos.org>

2008/12/20 James Cloos <cloos@jhcloos.com>:
>
> #!/bin/bash
> #
> # does this git repo need a pull?
> #
> l=$(git log|head -1|awk '{print $NF}')
> r=$(git ls-remote origin heads/master|awk '{print $1}')
> test "${r}" != "${l}"
>
>
> -JimC
> --
> James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6
>
>

Hello

Your script will report false positives if you run that in a branch
where you've made local commits since git log's output will list a
commit that's not on the remote side.  Thus it'lll say 'you need to
pull' when really what happened was that you committed locally and the
sha1 test is no longer equal.  This is one of the reasons why it's a
good idea to always keep your local master clean.

There's two things you can do:

1. assume that you always keep your local 'master' branch clean.
That'll let you quickly compare your local master versus origin's
master:

#!/bin/sh
remotemaster=$(git ls-remote origin heads/master | awk '{print $1}')
localmaster=$(git rev-parse master)
test "$remotemaster" = "$localmaster"


2. You might have a local branch that's not called master but is
tracking origin/master.  Or, your master branch might not be clean.
You can accomodate that workflow with:

#!/bin/sh
remotemaster=$(git ls-remote origin heads/master | awk '{print $1}')
branchpoint=$(git merge-base origin/master HEAD)
test "$remotemaster" = "$branchpoint"


The difference is that we're checking against the branch point.  If
origin/master is beyond the branch point then chances are you need to
pull.



---- off topic, but related ----

BTW one sound recommendation that I've heard on this list is that your
topic branches should really be free of any unrelated changes.
Pulling stuff in "just because" changes the branch.  It's no longer
just "topic" -- it's now "topic" + whatever they happened to push
upstream.  That has some implications in that it makes it harder to
review what exactly went into the specific topic/feature since the
branch's history now contains unrelated changes.

An advantage of keeping your topic branches clean is that you can run:

    git diff $(git merge-base HEAD origin/master)

in your topic branch and you'll see *only* the changes that went into
that branch.  If you get into the habit of pulling stuff in then
you'll see your changes *and* stuff you've pulled in, which probably
has nothing to do with the original topic/feature, etc. etc.

Anyways, this is an entirely different topic/conversation and might
not apply to your specific circumstance but I figured I'd mention it
nonetheless.


-- 
    David

^ permalink raw reply

* Re: how to check remote git repo for updates without pull/fetch
From: Boyd Stephen Smith Jr. @ 2008-12-21  0:02 UTC (permalink / raw)
  To: git; +Cc: David Aguilar, James Cloos, Ivan Zorin
In-Reply-To: <402731c90812201541r510170tbe1d56b7261e8146@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On Saturday 2008 December 20 17:41:13 David Aguilar wrote:
> 2008/12/20 James Cloos <cloos@jhcloos.com>:
> > #!/bin/bash
> > #
> > # does this git repo need a pull?
> > #
> > l=$(git log|head -1|awk '{print $NF}')
> > r=$(git ls-remote origin heads/master|awk '{print $1}')
> > test "${r}" != "${l}"
> >
> >
> > -JimC
> > --
> > James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6
>
> Hello
>
> Your script will report false positives if you run that in a branch
> where you've made local commits since git log's output will list a
> commit that's not on the remote side.

Change the last line to:
test "${r}" != "$(git merge-base "${r}" "${l}")" and those false positives 
should go away.

The script is far from a complete solution to the OPs problem though.  It only 
checks one, specific remote branch.  It only checks against the current 
branch.  It is the core of something that might be useful as another 
subcommand to 'git remote' though.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [PATCH] Have manpage reference new documentation on reverting merges.
From: Boyd Stephen Smith Jr. @ 2008-12-21  0:32 UTC (permalink / raw)
  To: git, Nanako Shiraishi, Junio C Hamano

Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
An example addition to the manpage for revert that references Nanako
Shiraishi's new documentation.

 Documentation/git-revert.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index caa0729..ea36bdf 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -45,6 +45,10 @@ OPTIONS
 	the mainline and allows revert to reverse the change
 	relative to the specified parent.
 
+	Reverting a merge commit does not completely "undo" the effect of the
+	merge and it may make future merges more difficult.  For more details,
+	please read Documentation/howto/revert-a-faulty-merge.txt.
+
 --no-edit::
 	With this option, 'git-revert' will not start the commit
 	message editor.
-- 
1.5.6
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

^ permalink raw reply related

* [PATCH] connect.c: stricter port validation, silence compiler warning
From: René Scharfe @ 2008-12-21  1:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

In addition to checking if the provided port is numeric, also check
that the string isn't empty and that the port number is within the
valid range.  Incidentally, this silences a compiler warning about
ignoring strtol's return value.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 connect.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index 584e04c..2f55ad2 100644
--- a/connect.c
+++ b/connect.c
@@ -480,8 +480,8 @@ char *get_port(char *host)
 	char *p = strchr(host, ':');
 
 	if (p) {
-		strtol(p+1, &end, 10);
-		if (*end == '\0') {
+		long port = strtol(p + 1, &end, 10);
+		if (end != p + 1 && *end == '\0' && 0 <= port && port < 65536) {
 			*p = '\0';
 			return p+1;
 		}
-- 
1.6.1.rc3.52.g589372

^ permalink raw reply related

* [PATCH] fast-import.c: stricter strtoul check, silence compiler warning
From: René Scharfe @ 2008-12-21  1:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

Store the return value of strtoul() in order to avoid compiler
warnings on Ubuntu 8.10.

Also check errno after each call, which is the only way to notice
an overflow without making ULONG_MAX an illegal date.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
I don't really like the first part, as we're ignoring the return
value anyway, even if we store it in the variable "date", so this
is quite useless.  But better to have a bit more useless code
than to see these equally useless warnings on every build.
Turning them off completely is not a good idea, since some of
them resulted in useful fixes (see 47d32af2, 304dcf26, 7be77de2).

 fast-import.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 171d178..a6bce66 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1748,18 +1748,21 @@ static int validate_raw_date(const char *src, char *result, int maxlen)
 {
 	const char *orig_src = src;
 	char *endp, sign;
+	unsigned long date;
 
-	strtoul(src, &endp, 10);
-	if (endp == src || *endp != ' ')
+	errno = 0;
+
+	date = strtoul(src, &endp, 10);
+	if (errno || endp == src || *endp != ' ')
 		return -1;
 
 	src = endp + 1;
 	if (*src != '-' && *src != '+')
 		return -1;
 	sign = *src;
 
-	strtoul(src + 1, &endp, 10);
-	if (endp == src || *endp || (endp - orig_src) >= maxlen)
+	date = strtoul(src + 1, &endp, 10);
+	if (errno || endp == src || *endp || (endp - orig_src) >= maxlen)
 		return -1;
 
 	strcpy(result, orig_src);
-- 
1.6.1.rc3.52.g589372

^ permalink raw reply related

* [PATCH 1/2] fast-import: add special mode; copy from parent.
From: Felipe Contreras @ 2008-12-21  2:11 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-import.txt |    1 +
 fast-import.c                     |   41 +++++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c2f483a..9d4231e 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -484,6 +484,7 @@ in octal.  Git only supports the following modes:
 * `160000`: A gitlink, SHA-1 of the object refers to a commit in
   another repository. Git links can only be specified by SHA or through
   a commit mark. They are used to implement submodules.
+* `-`: A special mode; copy the parent's mode.
 
 In both formats `<path>` is the complete path of the file to be added
 (if not already existing) or modified (if already existing).
diff --git a/fast-import.c b/fast-import.c
index 3c035a5..a00d3fe 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1356,7 +1356,7 @@ static int tree_content_set(
 	struct tree_entry *root,
 	const char *p,
 	const unsigned char *sha1,
-	const uint16_t mode,
+	uint16_t mode,
 	struct tree_content *subtree)
 {
 	struct tree_content *t = root->tree;
@@ -1382,7 +1382,9 @@ static int tree_content_set(
 						&& e->versions[1].mode == mode
 						&& !hashcmp(e->versions[1].sha1, sha1))
 					return 0;
-				e->versions[1].mode = mode;
+				if (mode == S_IFREG)
+					mode = e->versions[0].mode;
+				e->versions[1].mode = (mode == S_IFREG) ? (S_IFREG | 0644) : mode;
 				hashcpy(e->versions[1].sha1, sha1);
 				if (e->tree)
 					release_tree_content_recursive(e->tree);
@@ -1417,7 +1419,7 @@ static int tree_content_set(
 		tree_content_set(e, slash1 + 1, sha1, mode, subtree);
 	} else {
 		e->tree = subtree;
-		e->versions[1].mode = mode;
+		e->versions[1].mode = (mode == S_IFREG) ? (S_IFREG | 0644) : mode;
 		hashcpy(e->versions[1].sha1, sha1);
 	}
 	hashclr(root->versions[1].sha1);
@@ -1862,20 +1864,25 @@ static void file_change_m(struct branch *b)
 	unsigned char sha1[20];
 	uint16_t mode, inline_data = 0;
 
-	p = get_mode(p, &mode);
-	if (!p)
-		die("Corrupt mode: %s", command_buf.buf);
-	switch (mode) {
-	case S_IFREG | 0644:
-	case S_IFREG | 0755:
-	case S_IFLNK:
-	case S_IFGITLINK:
-	case 0644:
-	case 0755:
-		/* ok */
-		break;
-	default:
-		die("Corrupt mode: %s", command_buf.buf);
+	if (!prefixcmp(p, "- ")) {
+		mode = 0;
+		p += 2;
+	} else {
+		p = get_mode(p, &mode);
+		if (!p)
+			die("Corrupt mode: %s", command_buf.buf);
+		switch (mode) {
+		case S_IFREG | 0644:
+		case S_IFREG | 0755:
+		case S_IFLNK:
+		case S_IFGITLINK:
+		case 0644:
+		case 0755:
+			/* ok */
+			break;
+		default:
+			die("Corrupt mode: %s", command_buf.buf);
+		}
 	}
 
 	if (*p == ':') {
-- 
1.6.0.6.3.g7ccbd

^ permalink raw reply related

* [PATCH 2/2] fast-import: add special '-' blob reference to use the previous one.
From: Felipe Contreras @ 2008-12-21  2:11 UTC (permalink / raw)
  To: git; +Cc: Felipe Contreras
In-Reply-To: <1229825502-963-1-git-send-email-felipe.contreras@gmail.com>

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-import.txt |    6 +++---
 fast-import.c                     |   13 ++++++++++---
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 9d4231e..6fce41f 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -457,9 +457,9 @@ External data format::
 	'M' SP <mode> SP <dataref> SP <path> LF
 ....
 +
-Here `<dataref>` can be either a mark reference (`:<idnum>`)
-set by a prior `blob` command, or a full 40-byte SHA-1 of an
-existing Git blob object.
+Here `<dataref>` can be either a mark reference (`:<idnum>`) set by a prior
+`blob` command, a special `-` reference to use the one of the ancestor, or
+a full 40-byte SHA-1 of an existing Git blob object.
 
 Inline data format::
 	The data content for the file has not been supplied yet.
diff --git a/fast-import.c b/fast-import.c
index a00d3fe..228c474 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1385,7 +1385,8 @@ static int tree_content_set(
 				if (mode == S_IFREG)
 					mode = e->versions[0].mode;
 				e->versions[1].mode = (mode == S_IFREG) ? (S_IFREG | 0644) : mode;
-				hashcpy(e->versions[1].sha1, sha1);
+				hashcpy(e->versions[1].sha1,
+					is_null_sha1(sha1) ? e->versions[0].sha1 : sha1);
 				if (e->tree)
 					release_tree_content_recursive(e->tree);
 				e->tree = subtree;
@@ -1420,7 +1421,8 @@ static int tree_content_set(
 	} else {
 		e->tree = subtree;
 		e->versions[1].mode = (mode == S_IFREG) ? (S_IFREG | 0644) : mode;
-		hashcpy(e->versions[1].sha1, sha1);
+		hashcpy(e->versions[1].sha1,
+			is_null_sha1(sha1) ? e->versions[0].sha1 : sha1);
 	}
 	hashclr(root->versions[1].sha1);
 	return 1;
@@ -1862,7 +1864,7 @@ static void file_change_m(struct branch *b)
 	const char *endp;
 	struct object_entry *oe = oe;
 	unsigned char sha1[20];
-	uint16_t mode, inline_data = 0;
+	uint16_t mode, inline_data = 0, empty_blob = 0;
 
 	if (!prefixcmp(p, "- ")) {
 		mode = 0;
@@ -1893,6 +1895,10 @@ static void file_change_m(struct branch *b)
 	} else if (!prefixcmp(p, "inline")) {
 		inline_data = 1;
 		p += 6;
+	} else if (!prefixcmp(p, "- ")) {
+		hashclr(sha1);
+		empty_blob = 1;
+		p += 1;
 	} else {
 		if (get_sha1_hex(p, sha1))
 			die("Invalid SHA1: %s", command_buf.buf);
@@ -1936,6 +1942,7 @@ static void file_change_m(struct branch *b)
 		if (oe->type != OBJ_BLOB)
 			die("Not a blob (actually a %s): %s",
 				typename(oe->type), command_buf.buf);
+	} else if (empty_blob) {
 	} else {
 		enum object_type type = sha1_object_info(sha1, NULL);
 		if (type < 0)
-- 
1.6.0.6.3.g7ccbd

^ permalink raw reply related

* Re: [PATCH] Add a documentat on how to revert a faulty merge
From: Junio C Hamano @ 2008-12-21  2:37 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: git, Nanako Shiraishi
In-Reply-To: <200812201612.37455.bss@iguanasuicide.net>

"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:

> On Saturday 2008 December 20 05:27:02 Nanako Shiraishi wrote:
>> +Date: Fri, 19 Dec 2008 00:45:19 -0800
>> +From: Linus Torvalds <torvalds@linux-foundation.org>, Junio C Hamano
>> <gitster@pobox.com> +Subject: Re: Odd merge behaviour involving reverts
>> +Abstract: Sometimes a branch that was already merged to the mainline
>> + is later found to be faulty.  Linus and Junio give guidance on
>> + recovering from such a premature merge and continuing development
>> + after the offending branch is fixed.
>> +Message-ID: <7vocz8a6zk.fsf@gitster.siamese.dyndns.org>
>> +References: <alpine.LFD.2.00.0812181949450.14014@localhost.localdomain>
>> +
>> +Alan <alan@clueserver.org> said:
>
> I don't like the email headers as part of the documentation.  It would be 
> better to have a title and abstract in prose.

The above follows the style of other files in Documentation/howto; if you
look at the build procedure of howto-index, you will notice that some of
these are required part of the documentation.

> Also, your email gave me some errors from 'git am':
> .dotest/patch:40: indent with spaces.
>                /

That is "C program source" rules in action.

I thought we had .gitattributes set up in Documentation/ area that applies
different rules to these?  Maybe they have not been tested with things in
howto.  

^ permalink raw reply

* Re: [PATCH] Have manpage reference new documentation on reverting merges.
From: Junio C Hamano @ 2008-12-21  2:36 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: git, Nanako Shiraishi
In-Reply-To: <200812201832.48992.bss@iguanasuicide.net>

"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:

> Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
> ---
> An example addition to the manpage for revert that references Nanako
> Shiraishi's new documentation.
>
>  Documentation/git-revert.txt |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
> index caa0729..ea36bdf 100644
> --- a/Documentation/git-revert.txt
> +++ b/Documentation/git-revert.txt
> @@ -45,6 +45,10 @@ OPTIONS
>  	the mainline and allows revert to reverse the change
>  	relative to the specified parent.
>  
> +	Reverting a merge commit does not completely "undo" the effect of the
> +	merge and it may make future merges more difficult.  For more details,
> +	please read Documentation/howto/revert-a-faulty-merge.txt.
> +

I think these new lines need to be dedented, and the previous blank line
should be turned into a line with a single '+'.

I'd also suggest removing "does not ... merge and it" from the above
sentence to avoid confusing readers, because people who read only the
above and do not read the howto document may get a wrong impression that
the resulting tree may have some changes that came from the merge even
after the revert, which is not the case.  Revert will erase the effect the
merge had to your tree and that part is complete.

Linus's "does not completely undo" only refers to the history part of the
merge, and that only affects future re-merges from the same branch, which
the reader who is interested in doing a revert of a merge right now (that
is why s/he is reading this paragraph) may not yet care about.

An alternative is to give a complete but brief explanation.  Perhaps like
this:

    By reverting a merge, you are declaring that you will never want the
    changes that were brought in by that merge you are reverting in your
    tree.  If you do merge from the same branch again in the future after
    it is updated, git remembers your declaration, and only the changes on
    the branch that were made after the reverted merge will be brought in.
    This may or may not be what you want.  See 'revert-a-faulty-merge'
    HOWTO for more details.

^ permalink raw reply

* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21  2:37 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <200812210031.08443.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:

> An alternative, would be "removing changes relative to .."
> (mainline). The changes are the contributions from all other parents. I
> have to huge interest in the exact phrase used.

But that is exactly what "This reverts commit X" means, isn't it?

^ permalink raw reply

* Re: Git weekly links: 2008-51
From: Junio C Hamano @ 2008-12-21  2:37 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git list
In-Reply-To: <94a0d4530812200416m1caa96f2je2bf478f65bd7d12@mail.gmail.com>

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

> This week tortoisegit stole the spotlight. Maybe there weren't many
> other links, or maybe I failed to notice them. Also, many people liked
> the comment of Linus Torvalds regarding C++ in git.

It seems that the week was quieter than usual, perhaps?

Many are rather old news.  People who know git world better, but do not
know how you are generating this list and for what purpose, may interpret
this as "Here are links I recommend you to follow and read this week" and
incorrectly think "why is this bozo listing these ancient news as this
week's?"

I do not think you would want that.  You may want to briefly mention at
the beginning of each issue (say two-line paragraph) how the links listed
here are chosen, primarily to explain that these are not hand-picked by
you, but culled from the public bookmarks, i.e. "what people are reading
this week" or something like that.

^ permalink raw reply

* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Boyd Stephen Smith Jr. @ 2008-12-21  3:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin Rosenberg, git
In-Reply-To: <7viqpetfs3.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

On Saturday 2008 December 20 20:37:16 Junio C Hamano wrote:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> > An alternative, would be "removing changes relative to .."
> > (mainline).
>
> But that is exactly what "This reverts commit X" means, isn't it?

When X is a merge commit, the phrase "the reverts commit X" is ambiguous.  Did 
you revert the tree to X^, X^2, or X^8?  I'd be fine with "This reverts 
commit X to X^y", but we definitely need some mention of X^y.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Have manpage reference new documentation on reverting merges.
From: Boyd Stephen Smith Jr. @ 2008-12-21  5:35 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nanako Shiraishi
In-Reply-To: <7vtz8ytft0.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 3303 bytes --]

On Saturday 2008 December 20 20:36:43 Junio C Hamano wrote:
> "Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> > ---
> > An example addition to the manpage for revert that references Nanako
> > Shiraishi's new documentation.
> >
> >  Documentation/git-revert.txt |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
> > index caa0729..ea36bdf 100644
> > --- a/Documentation/git-revert.txt
> > +++ b/Documentation/git-revert.txt
> > @@ -45,6 +45,10 @@ OPTIONS
> >  	the mainline and allows revert to reverse the change
> >  	relative to the specified parent.
> >
> > +	Reverting a merge commit does not completely "undo" the effect of the
> > +	merge and it may make future merges more difficult.  For more details,
> > +	please read Documentation/howto/revert-a-faulty-merge.txt.
> > +
>
> I think these new lines need to be dedented, and the previous blank line
> should be turned into a line with a single '+'.

Can do.

> I'd also suggest removing "does not ... merge and it" from the above
> sentence to avoid confusing readers, because people who read only the
> above and do not read the howto document may get a wrong impression that
> the resulting tree may have some changes that came from the merge even
> after the revert, which is not the case.  Revert will erase the effect the
> merge had to your tree and that part is complete.

But that is not the whole effect of the merge.  The effect of the merge is 
both the modifications it makes to the tree and the modifications it makes to 
the history.

Going from the dictionary meaning for "revert", one might expect the those 
effects to go away as well.  I think a warning that the revert subcommand 
does not fully revert the merge is appropriate.

> Linus's "does not completely undo" only refers to the history part of the
> merge, and that only affects future re-merges from the same branch, which
> the reader who is interested in doing a revert of a merge right now (that
> is why s/he is reading this paragraph) may not yet care about.

They may not care about it now, but it doesn't make much sense to warn about 
it during the later merge (plus it might be computationally expensive to 
detect).

> An alternative is to give a complete but brief explanation.  Perhaps like
> this:
>
>     By reverting a merge, you are declaring that you will never want the
>     changes that were brought in by that merge you are reverting in your
>     tree.  If you do merge from the same branch again in the future after
>     it is updated, git remembers your declaration, and only the changes on
>     the branch that were made after the reverted merge will be brought in.
>     This may or may not be what you want.  See 'revert-a-faulty-merge'
>     HOWTO for more details.

I think the wording might need to be changed a little bit, but I do like the 
longer, more complete and clear explanation and I'll work on a patch that has 
one.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Memory issue with fast-import, why track branches?
From: Felipe Contreras @ 2008-12-21  5:54 UTC (permalink / raw)
  To: git list

Hi,

I tracked down an issue I have when importing a big repository. For
some reason memory usage keeps increasing until there is no more
memory.

Here is what valgrind shows:
==21034== 471,080,280 bytes in 114,517 blocks are still reachable in
loss record 8 of 8
==21034==    at 0x4004BA2: calloc (vg_replace_malloc.c:397)
==21034==    by 0x806A340: xcalloc (wrapper.c:75)
==21034==    by 0x8063BC1: use_pack (sha1_file.c:808)
==21034==    by 0x8063DA9: unpack_object_header (sha1_file.c:1443)
==21034==    by 0x8064F4F: unpack_entry (sha1_file.c:1736)
==21034==    by 0x8065393: cache_or_unpack_entry (sha1_file.c:1606)
==21034==    by 0x8065464: read_packed_sha1 (sha1_file.c:2000)
==21034==    by 0x80655E5: read_object (sha1_file.c:2090)
==21034==    by 0x8065677: read_sha1_file (sha1_file.c:2106)
==21034==    by 0x8056AE9: parse_object (object.c:190)
==21034==    by 0x805E90A: write_ref_sha1 (refs.c:1214)
==21034==    by 0x804CC4F: update_branch (fast-import.c:1558)

After looking at the code my guess is that I have a humongous amount
of branches.

Actually they are not really branches, but refs. For each git commit
there's an original mtn ref that I store in 'refs/mtn/sha1', but since
I'm using 'commit refs/mtn/sha1' to store it, a branch is created for
every commit.

I guess there are many ways to fix the issue, but for starters I
wonder why is fast-import keeping track of all the branches? In my
case I would like fast-import to work exactly the same if I specify
branches or not (I'll update them later).

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: git-svn and empty directories
From: Eric Wong @ 2008-12-21  7:08 UTC (permalink / raw)
  To: Thomas Jarosch; +Cc: Deskin Miller, git
In-Reply-To: <200812161353.49796.thomas.jarosch@intra2net.com>

Thomas Jarosch <thomas.jarosch@intra2net.com> wrote:
> Hello Eric and Deskin,
> 
> I'm currently looking into preserving empty directories from a SVN repository 
> by automatically creating empty .gitignore files for them.
> 
> The control flow of the git-svn code is still a jungle to me,
> maybe you have a hint how to implement a proof-of-concept code?
> 
> I don't think I can just touch a .gitignore file in get_untracked()
> and those files will magically turn up in git's index...

Hi Thomas,

Modern git-svn never touches the working tree during fetch, it hashes
objects into the database and adds those to the indexes directly.

However, I don't think your proposal is a good idea since it adds too
much "magic".  Complex special cases for delta application if the
.gitignore gets real content and backwards-incompatibility since I know
some git-svn users already rely on pushing .gitignore files (empty or
otherwise) to an upstream SVN repo.

The minor problem of missing empty directories isn't big enough to be
worth the trouble IMHO.

The unhandled.log is made to be machine parseable, so if somebody really
wanted to recreate empty direct after checkout, they could write a
script that parses it and creates it based on the history of the current
working tree.

-- 
Eric Wong

^ permalink raw reply

* Re: Memory issue with fast-import, why track branches?
From: John Chapman @ 2008-12-21  8:10 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git list
In-Reply-To: <94a0d4530812202154l26dfe0dfm49397c63dbfdfdf9@mail.gmail.com>

My first response was along the lines of "Why the heck are you storing
sha1's like that!?", until I realised that you're not storing actual git
sha1's, but mtn's hashes, which does make sense.

I'm doing something very similar with my perforce scripts, however I am
doing a bit more magic instead of making so many branches.

Instead of making branches, I make a tag instead, for each and every
changeset.  Every time I make a new git commit, if I need to do it from
a tag, I first read the tag and determine the sha1 I should use, and use
that instead.

Alternatively, you could choose to manage your mapping yourself, and
write them to a .git/mtg-git-map file.

On Sun, 2008-12-21 at 07:54 +0200, Felipe Contreras wrote:
> Hi,
> 
> I tracked down an issue I have when importing a big repository. For
> some reason memory usage keeps increasing until there is no more
> memory.
> 
> Here is what valgrind shows:
> ==21034== 471,080,280 bytes in 114,517 blocks are still reachable in
> loss record 8 of 8
> ==21034==    at 0x4004BA2: calloc (vg_replace_malloc.c:397)
> ==21034==    by 0x806A340: xcalloc (wrapper.c:75)
> ==21034==    by 0x8063BC1: use_pack (sha1_file.c:808)
> ==21034==    by 0x8063DA9: unpack_object_header (sha1_file.c:1443)
> ==21034==    by 0x8064F4F: unpack_entry (sha1_file.c:1736)
> ==21034==    by 0x8065393: cache_or_unpack_entry (sha1_file.c:1606)
> ==21034==    by 0x8065464: read_packed_sha1 (sha1_file.c:2000)
> ==21034==    by 0x80655E5: read_object (sha1_file.c:2090)
> ==21034==    by 0x8065677: read_sha1_file (sha1_file.c:2106)
> ==21034==    by 0x8056AE9: parse_object (object.c:190)
> ==21034==    by 0x805E90A: write_ref_sha1 (refs.c:1214)
> ==21034==    by 0x804CC4F: update_branch (fast-import.c:1558)
> 
> After looking at the code my guess is that I have a humongous amount
> of branches.
> 
> Actually they are not really branches, but refs. For each git commit
> there's an original mtn ref that I store in 'refs/mtn/sha1', but since
> I'm using 'commit refs/mtn/sha1' to store it, a branch is created for
> every commit.
> 
> I guess there are many ways to fix the issue, but for starters I
> wonder why is fast-import keeping track of all the branches? In my
> case I would like fast-import to work exactly the same if I specify
> branches or not (I'll update them later).
> 
> Cheers.
> 

^ permalink raw reply

* Re: [PATCH] Have manpage reference new documentation on reverting merges.
From: Junio C Hamano @ 2008-12-21  8:00 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: git, Nanako Shiraishi
In-Reply-To: <200812202335.19006.bss@iguanasuicide.net>

"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:

> But that is not the whole effect of the merge.  The effect of the merge is 
> both the modifications it makes to the tree and the modifications it makes to 
> the history.
>
> Going from the dictionary meaning for "revert", one might expect the those 
> effects to go away as well.  I think a warning that the revert subcommand 
> does not fully revert the merge is appropriate.

You may be technically correct, but think about the intended audience of
the warning.

People who know there are the "tree" aspect and the "history" aspect in
the merge and the revert operation will not get confused by the issue we
describe in the new HOW-TO, and they will understand the ramifications
without the help from the warning in your new paragraph.  The new warning
is trying to help people who do not understand these two aspects..

I have a strong suspicion that the "tree" aspect is much easier to grasp
by new people, while the understanding of the "history" aspect comes much
later when one becomes more proficient in using git.  Also, the immediate
interest of people who are reading "git revert" manual page is to revert
the effect of the merge in the "tree" aspect (iow, "I want to get the damn
thing work again"); that is where the focus of their attention is when
they read this manual page.

Your "not completely" does not tell them that the incompleteness you talk
about is "tree aspect only, not history aspect".  That is what I meant by
"... may get a wrong impression".  Their attention is about the "tree"
aspect, and your "not completely" will be easily misinterpreted as "the
tree effect of the merge won't get reverted completely", which is not what
you want to say.  And that is why I think you are much better off not
saying "not completely" if you do not explain what you mean by it.

As I showed you how in the previous message, an alternative is to say (the
equivalent of) "not completely", but explain what incompleteness you mean.

>> Linus's "does not completely undo" only refers to the history part of the
>> merge, and that only affects future re-merges from the same branch, which
>> the reader who is interested in doing a revert of a merge right now (that
>> is why s/he is reading this paragraph) may not yet care about.
>
> They may not care about it now, but it doesn't make much sense to warn about 
> it during the later merge (plus it might be computationally expensive to 
> detect).

Who's talking about giving a warning by computation?  Please stay on the
discussion of your documentation patch.

>> An alternative is to give a complete but brief explanation.  Perhaps like
>> this:
>>
>>     By reverting a merge, you are declaring that you will never want the
>>     changes that were brought in by that merge you are reverting in your
>>     tree.  If you do merge from the same branch again in the future after
>>     it is updated, git remembers your declaration, and only the changes on
>>     the branch that were made after the reverted merge will be brought in.
>>     This may or may not be what you want.  See 'revert-a-faulty-merge'
>>     HOWTO for more details.
>
> I think the wording might need to be changed a little bit, but I do like the 
> longer, more complete and clear explanation and I'll work on a patch that has 
> one.

^ permalink raw reply

* Re: Applying patches from a patch set
From: Sitaram Chamarty @ 2008-12-21  8:20 UTC (permalink / raw)
  To: git
In-Reply-To: <dac45060812200637m49c71aa5x3c25010fa00f4a63@mail.gmail.com>

On 2008-12-20, Mark Ryden <markryde@gmail.com> wrote:

>   I am subscribed to some linux kernel subsystem mailing list; in this
> list there are sometimes patchsets with more than
> 30-40 patches.
> I am using gmail web interface client.

solution 1: see if that list is mirrored by gmane and use a
newsreader like slrn to access the list via gmane

solution 2: enable pop/imap access on your gmail account and
pull emails from there using whatever command line mail
client you like (like mutt maybe).  Not tested, but should
work.  For some definition of "work" (not sure how gmail's
"tags" map to imap folders, which might trip you...)

^ permalink raw reply

* Re: [PATCH] handle_remote_ls_ctx can parsing href starting at http://
From: Junio C Hamano @ 2008-12-21  9:42 UTC (permalink / raw)
  To: Kirill A. Korinskiy; +Cc: git
In-Reply-To: <1229772213-11932-1-git-send-email-catap@catap.ru>

"Kirill A. Korinskiy" <catap@catap.ru> writes:

> The program call remote_ls() to get remote objects over http;
> handle_remote_ls_ctx() is used to parse it's response to populated
> "struct remote_ls_ctx" that is returned from remote_ls().
>
> The handle_remote_ls_ctx() function assumed that the server will
> returned local path in href field, but RFC 4918 demand of support full
> URI (http://localhost/repo.git for example).

Do you mean "the client should support both server-relative '/repo.git'
and full 'http://localhost/repo.git'", or "the client should reject
'/repo.git' and insist on full 'http://localhost/repo.git'"?  I am
guessing the former but it is not quite clear.  Where in 4918 is this
specified?

> This resulted in push failure (git-http-push ask server
> PROPFIND /repo.git/alhost:8080/repo.git/refs/) when a server returned
> full URI.

This is an interesting but confusing example.

Do you mean the bug is:

 (1) the client asks PROPFIND /repo.git/;

 (2) the server gives http://localhost/repo.git/refs back;

 (3) the client incorrectly assumes that the response would start with
     /repo.git/ (e.g. "/repo.git/refs"), so strips 10 bytes from the
     beginning of this result and uses the remainder as the "new"
     information to dig deeper; i.e. "alhost/repo.git/refs";

 (4) the new part is appended to the original path and the client forms
     the next request "PROPFIND /repo.git/alhost/repo.git/refs/";

 (5) instead, the client should strip the proto://host part (if exists)
     and request "PROPFIND /repo.git/refs/".

> @@ -1424,9 +1425,10 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
>  				ls->userFunc(ls);
>  			}
>  		} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> -			ls->dentry_name = xmalloc(strlen(ctx->cdata) -
> +			char *path = strstr(ctx->cdata, remote->path);
> +			ls->dentry_name = xmalloc(strlen(path) -
>  						  remote->path_len + 1);

What if you are talking to http://repo.git/repo.git/?  Doesn't this
strstr() misbehave?  Instead, shouldn't you be checking if the response
begins with proto://host/ and stripping it iff so?

^ permalink raw reply

* Re: how to check remote git repo for updates without pull/fetch
From: James Cloos @ 2008-12-21  9:53 UTC (permalink / raw)
  To: David Aguilar; +Cc: Ivan Zorin, git
In-Reply-To: <402731c90812201541r510170tbe1d56b7261e8146@mail.gmail.com>

>>>>> "David" == David Aguilar <davvid@gmail.com> writes:

David> Your script will report false positives if you run that in a branch
David> where you've made local commits since git log's output will list a
David> commit that's not on the remote side.

Ah.  Yes.  I forgot to mention that.  Good catch.

I wrote the script specifically for use with the repos created by Gentoo's
git eclass (used when installing packages from git repos rather than from
tars or the like).  As such, it only needed to deal with pristine clones.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

^ permalink raw reply

* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Robin Rosenberg @ 2008-12-21 10:09 UTC (permalink / raw)
  To: Boyd Stephen Smith Jr.; +Cc: Junio C Hamano, git
In-Reply-To: <200812202111.17831.bss@iguanasuicide.net>

söndag 21 december 2008 04:11:13 skrev Boyd Stephen Smith Jr.:
> On Saturday 2008 December 20 20:37:16 Junio C Hamano wrote:
> > Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
> > > An alternative, would be "removing changes relative to .."
> > > (mainline).
> >
> > But that is exactly what "This reverts commit X" means, isn't it?
> 
> When X is a merge commit, the phrase "the reverts commit X" is ambiguous.  Did 
> you revert the tree to X^, X^2, or X^8?  I'd be fine with "This reverts 
> commit X to X^y", but we definitely need some mention of X^y.

One could consider keeping the contributions from ^1 a special case and not
mention the parent, making it look like any revert commit. I guess most merge
reverts are like this in practice.

-- robin

^ permalink raw reply

* Re: [PATCH] Make git revert warn the user when reverting a merge commit.
From: Junio C Hamano @ 2008-12-21 10:59 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Boyd Stephen Smith Jr., git
In-Reply-To: <200812211109.36788.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:

> One could consider keeping the contributions from ^1 a special case and not
> mention the parent, making it look like any revert commit. I guess most merge
> reverts are like this in practice.

I think that makes sense.  There are cases where the mainline maintainer
punts a merge and pass the baton to a subsystem maintainer, saying "Your
tree has many conflicts with my tip, and I'd rather ask you to resolve it"
(and after such a merge, the mainline maintainer will fast forward to the
result), in which case the merge will be in the reverse direction, but
that should be rare.  Reverting such a merge later from the mainline's
point of view would involve "revert -m 2".

So if your patch is tightened a bit to record extra information only in
such a case, I think that would be an acceptable approach to the issue.

^ permalink raw reply

* Re: Memory issue with fast-import, why track branches?
From: Felipe Contreras @ 2008-12-21 11:23 UTC (permalink / raw)
  To: John Chapman; +Cc: git list
In-Reply-To: <1229847042.798.5.camel@therock.nsw.bigpond.net.au>

On Sun, Dec 21, 2008 at 10:10 AM, John Chapman <thestar@fussycoder.id.au> wrote:
> My first response was along the lines of "Why the heck are you storing
> sha1's like that!?", until I realised that you're not storing actual git
> sha1's, but mtn's hashes, which does make sense.

Yes :)

> I'm doing something very similar with my perforce scripts, however I am
> doing a bit more magic instead of making so many branches.
>
> Instead of making branches, I make a tag instead, for each and every
> changeset.  Every time I make a new git commit, if I need to do it from
> a tag, I first read the tag and determine the sha1 I should use, and use
> that instead.

Well, simple tags and branches are exactly the same thing: refs. tags
are in 'refs/tags' and branches in 'refs/heads'; 'refs/mtn' are not
really branches.

> Alternatively, you could choose to manage your mapping yourself, and
> write them to a .git/mtg-git-map file.

The advantage of my approach is that the git tools handle all the mtn
sha1's almost as good as git sha1's, I just need to prepend 'mtn/'.

Also, git name-rev finds the mtn revision of a git commit. It' all so
convenient.

The only problem is that fast-import seems to be doing something wrong
with those "branches".

-- 
Felipe Contreras

^ 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