Git development
 help / color / mirror / Atom feed
* [PATCH jn/fast-import-fix v3] fast-import: treat filemodify with empty tree as delete
From: Jonathan Nieder @ 2011-01-26 23:06 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Git Mailing List, Ramkumar Ramachandra,
	Shawn O. Pearce, David Barr
In-Reply-To: <AANLkTimNWLFgTk0Bueiscw-WkAX53v0Xsepn9esXOt7+@mail.gmail.com>

Date: Sat, 11 Dec 2010 16:42:28 -0600

Normal git processes do not allow one to build a tree with an empty
subtree entry without trying hard at it.  This is in keeping with the
general UI philosophy: git tracks content, not empty directories.

Unfortunately, v1.7.3-rc0~75^2 (2010-06-30) changed that by making it
easy to include an empty subtree in fast-import's active commit:

	M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir

It is easy to trigger this by accident by reading an empty tree (for
example, the tree corresponding to an empty root commit) and trying to
move it to a subtree.  It would be better and more closely analogous
to "git read-tree --prefix" to treat such commands as a request to
remove ("to empty") the subdir.

Noticed-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Sverre Rabbelier wrote:

> Should it go on maint now that it's factored out, since it shipped in
> 1.7.3, or just master?

Hmm.  I suppose on top of b2124125 (jn/fast-import-fix).

While applying it there I noticed that the change to t9300 includes an
unrelated change (residue of an old rebase).  Here's a fixed version.

 fast-import.c          |   10 ++++++++++
 t/t9300-fast-import.sh |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index d881630..9cf26f1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2194,6 +2194,16 @@ static void file_change_m(struct branch *b)
 		p = uq.buf;
 	}
 
+	/*
+	 * Git does not track empty, non-toplevel directories.
+	 */
+	if (S_ISDIR(mode) &&
+	    !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) &&
+	    *p) {
+		tree_content_remove(&b->branch_tree, p, NULL);
+		return;
+	}
+
 	if (S_ISGITLINK(mode)) {
 		if (inline_data)
 			die("Git links cannot be specified 'inline': %s",
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index dd90a09..ef3a347 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -875,6 +875,48 @@ test_expect_success \
 	 compare_diff_raw expect actual'
 
+test_expect_success \
+	'N: delete directory by copying' \
+	'cat >expect <<-\EOF &&
+	OBJID
+	:100644 000000 OBJID OBJID D	foo/bar/qux
+	OBJID
+	:000000 100644 OBJID OBJID A	foo/bar/baz
+	:000000 100644 OBJID OBJID A	foo/bar/qux
+	EOF
+	 empty_tree=$(git mktree </dev/null) &&
+	 cat >input <<-INPUT_END &&
+	commit refs/heads/N-delete
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	collect data to be deleted
+	COMMIT
+
+	deleteall
+	M 100644 inline foo/bar/baz
+	data <<DATA_END
+	hello
+	DATA_END
+	C "foo/bar/baz" "foo/bar/qux"
+	C "foo/bar/baz" "foo/bar/quux/1"
+	C "foo/bar/baz" "foo/bar/quuux"
+	M 040000 $empty_tree foo/bar/quux
+	M 040000 $empty_tree foo/bar/quuux
+
+	commit refs/heads/N-delete
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	delete subdirectory
+	COMMIT
+
+	M 040000 $empty_tree foo/bar/qux
+	INPUT_END
+	 git fast-import <input &&
+	 git rev-list N-delete |
+		git diff-tree -r --stdin --root --always |
+		sed -e "s/$_x40/OBJID/g" >actual &&
+	 test_cmp expect actual'
+
 test_expect_success \
 	'N: copy root directory by tree hash' \
 	'cat >expect <<-\EOF &&
 	:100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D	file3/newf
-- 
1.7.4.rc3

^ permalink raw reply related

* Re: [PATCH v2] fast-import: treat filemodify with empty tree as delete
From: Sverre Rabbelier @ 2011-01-26 22:45 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Git Mailing List, Ramkumar Ramachandra,
	Shawn O. Pearce, David Barr
In-Reply-To: <20110126224104.GA20388@burratino>

Heya,

On Wed, Jan 26, 2011 at 23:41, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Seems to work okay. :)

Should it go on maint now that it's factored out, since it shipped in
1.7.3, or just master?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH v2] fast-import: treat filemodify with empty tree as delete
From: Jonathan Nieder @ 2011-01-26 22:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Ramkumar Ramachandra, Sverre Rabbelier,
	Shawn O. Pearce, David Barr
In-Reply-To: <20110103082458.GC8842@burratino>

Date: Sat, 11 Dec 2010 16:42:28 -0600

Normal git processes do not allow one to build a tree with an empty
subtree entry without trying hard at it.  This is in keeping with the
general UI philosophy: git tracks content, not empty directories.

v1.7.3-rc0~75^2 (2010-06-30) changed that by making it easy to include
an empty subtree in fast-import's active commit:

	M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 subdir

It is easy to trigger this by accident by reading an empty tree (for
example, the tree corresponding to an empty root commit) and trying to
move it to a subtree.  It is better and more closely analogous to "git
read-tree --prefix" to treat such commands as a request to remove
("to empty") the subdir.

Noticed-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Change since v1: commit message.

Resubmitting this fix separately from the 3-part series it came from.
Seems to work okay. :)

 fast-import.c          |   10 ++++++++
 t/t9300-fast-import.sh |   58 +++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 7857760..8b19d87 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2231,6 +2231,16 @@ static void file_change_m(struct branch *b)
 		p = uq.buf;
 	}
 
+	/*
+	 * Git does not track empty, non-toplevel directories.
+	 */
+	if (S_ISDIR(mode) &&
+	    !memcmp(sha1, (const unsigned char *) EMPTY_TREE_SHA1_BIN, 20) &&
+	    *p) {
+		tree_content_remove(&b->branch_tree, p, NULL);
+		return;
+	}
+
 	if (S_ISGITLINK(mode)) {
 		if (inline_data)
 			die("Git links cannot be specified 'inline': %s",
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 222d105..80ddfe0 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -42,6 +42,14 @@ echo "$@"'
 
 >empty
 
+test_expect_success 'setup: have pipes?' '
+	rm -f frob &&
+	if mkfifo frob
+	then
+		test_set_prereq PIPE
+	fi
+'
+
 ###
 ### series A
 ###
@@ -899,6 +907,48 @@ test_expect_success \
 	 compare_diff_raw expect actual'
 
+test_expect_success \
+	'N: delete directory by copying' \
+	'cat >expect <<-\EOF &&
+	OBJID
+	:100644 000000 OBJID OBJID D	foo/bar/qux
+	OBJID
+	:000000 100644 OBJID OBJID A	foo/bar/baz
+	:000000 100644 OBJID OBJID A	foo/bar/qux
+	EOF
+	 empty_tree=$(git mktree </dev/null) &&
+	 cat >input <<-INPUT_END &&
+	commit refs/heads/N-delete
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	collect data to be deleted
+	COMMIT
+
+	deleteall
+	M 100644 inline foo/bar/baz
+	data <<DATA_END
+	hello
+	DATA_END
+	C "foo/bar/baz" "foo/bar/qux"
+	C "foo/bar/baz" "foo/bar/quux/1"
+	C "foo/bar/baz" "foo/bar/quuux"
+	M 040000 $empty_tree foo/bar/quux
+	M 040000 $empty_tree foo/bar/quuux
+
+	commit refs/heads/N-delete
+	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+	data <<COMMIT
+	delete subdirectory
+	COMMIT
+
+	M 040000 $empty_tree foo/bar/qux
+	INPUT_END
+	 git fast-import <input &&
+	 git rev-list N-delete |
+		git diff-tree -r --stdin --root --always |
+		sed -e "s/$_x40/OBJID/g" >actual &&
+	 test_cmp expect actual'
+
 test_expect_success \
 	'N: copy root directory by tree hash' \
 	'cat >expect <<-\EOF &&
 	:100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D	file3/newf
@@ -1889,14 +1939,6 @@ test_expect_success 'R: print two blobs to stdout' '
 	test_cmp expect actual
 '
 
-test_expect_success 'setup: have pipes?' '
-	rm -f frob &&
-	if mkfifo frob
-	then
-		test_set_prereq PIPE
-	fi
-'
-
 test_expect_success PIPE 'R: copy using cat-file' '
 	expect_id=$(git hash-object big) &&
 	expect_len=$(wc -c <big) &&
-- 
1.7.4.rc3

^ permalink raw reply related

* Re: [msysGit] Git unable to access https repositories due to curl/OpenSSL 1.0.0 handshake issues
From: Mika Fischer @ 2011-01-26 22:18 UTC (permalink / raw)
  To: Daniel Stenberg; +Cc: kusmabite, msysGit, Git Mailing List
In-Reply-To: <alpine.DEB.2.00.1101262244550.14066@tvnag.unkk.fr>

On Wed, Jan 26, 2011 at 23:00, Daniel Stenberg <daniel@haxx.se> wrote:
>> curl/openssl 0.9.8k <-> apache/openssl 1.0.0 -> error
>
>> I'm not sure what to take away from this. Maybe it's a problem that is
>> partly caused by both apache and curl?
>
> Could be. I just have a hard time to see why code in curl that has worked
> for so many years suddenly would stop working. It just so feels like else
> changed.

Well, it's definitely a possibility that neither Apache nor Curl are
at fault but OpenSSL. The strange thing is that I could not produce
any failure with the OpenSSL command line tool. But that doesn't mean
that openssl is completely innocent.

> This error (or something similar to it) is often seen when we try to talk
> plain HTTP to a HTTPS server or vice versa. Did you verify that HTTPS was
> working fine on that port when you ran the OpenSSL 1.0.0 version of the
> server?

No, it definitely worked before with curl/openssl 1.0.0 and all kinds
of browsers. Also, as I wrote, it worked with the OpenSSL 0.9.8k
command line tool.

> Perhaps it is possible to add verbose level and further debug log stuff in
> the server to see what makes it suddenly decide the handshake is bad.

I didn't find much in this direction in the apache docs, unfortunately...
http://httpd.apache.org/docs/2.2/mod/mod_ssl.html

Next thing I'll try is to check whether I can reproduce this using a
different distribution that also uses OpenSSL 1.0.0. I'll let you know
when I have the results...

Best,
 Mika

^ permalink raw reply

* Re: Updating a submodule with a compatible version from another submodule version using the parent meta-repository
From: Junio C Hamano @ 2011-01-26 22:05 UTC (permalink / raw)
  To: Julian Ibarz; +Cc: Jens Lehmann, git
In-Reply-To: <AANLkTik8VrhbBSLwRq9gd39hofnmifk15zSqXVTsSzAp@mail.gmail.com>

Julian Ibarz <julian.ibarz@gmail.com> writes:

> On Wed, Jan 26, 2011 at 3:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Julian Ibarz <julian.ibarz@gmail.com> writes:
>>
>>>> Hmm, looks like I lost you here ... you want to bisect in B although
>>>> you know what commit you want there? Care to explain a bit more?
>>>
>>> In B I have a feature to integrate in master branch. This feature is
>>> in branch old_feature. But this branch is really old. To try this
>>> feature I need to rebuild it at this version. To make the build
>>> success I need also to revert back the submodule C because B is
>>> dependent on it. But finding the good version of C that match
>>> old_feature version is a pain... Is it clear?
>>
>> That sounds like bisecting in C with a frozen checkout of B to see which
>> version in C works well with that target commit in B you know you want to
>> use.  Why do you need to bisect in B???
>
> Forget about bisect. This is a different use case where I need the
> feature I am talking about: checkout an old version in B and
> automatically having A and C switch to a compatible version (the cause
> can be because of a bisect or just because I want to try an old
> feature not yet integrated into master or whatever the reason I want
> to be on this old version).

If that version of submodule B is explicitly bound to a commit in the
superproject A, you know which version of A and C were recorded, and the
problem is solved.

But otherwise, you are wishing for a miracle, I am afraid, without giving
your tool (git or your own tool you write on top of it) some hint to say
how "similar" commits in B are, as there is no guarantee that anybody even
tried a commit that is not directly referenced from the superproject
together with other parts of the system.

If you are confident that you didn't introduce different kind of
dependency to other submodules while developing your "old_feature" branch
in submodule B, one strategy may be to find an ancestor, preferrably the
fork point, of your "old_feature" branch that is bound to the superproject
A.  Then at that point at least you know whoever made that commit in A
tested the combination of what was recorded in that commit, together with
the version of B and C, and you can go forward from there, replaying the
changes you made to the "old_feature" branch in submodule B.

^ permalink raw reply

* Re: [msysGit] Git unable to access https repositories due to curl/OpenSSL 1.0.0 handshake issues
From: Daniel Stenberg @ 2011-01-26 22:00 UTC (permalink / raw)
  To: Mika Fischer; +Cc: kusmabite, msysGit, Git Mailing List
In-Reply-To: <AANLkTi=Gey75oo-iaELVWg87cP+HgM3RQ60vPgwaEMpS@mail.gmail.com>

On Wed, 26 Jan 2011, Mika Fischer wrote:

> curl/openssl 0.9.8k <-> apache/openssl 1.0.0 -> error

> I'm not sure what to take away from this. Maybe it's a problem that is 
> partly caused by both apache and curl?

Could be. I just have a hard time to see why code in curl that has worked for 
so many years suddenly would stop working. It just so feels like else changed.

This error (or something similar to it) is often seen when we try to talk 
plain HTTP to a HTTPS server or vice versa. Did you verify that HTTPS was 
working fine on that port when you ran the OpenSSL 1.0.0 version of the 
server?

Perhaps it is possible to add verbose level and further debug log stuff in the 
server to see what makes it suddenly decide the handshake is bad.

-- 

  / daniel.haxx.se

^ permalink raw reply

* Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"
From: Junio C Hamano @ 2011-01-26 21:43 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Adam Tkac, git, Jochen Schmitt
In-Reply-To: <m339ofbb9t.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

>> Current highlight utility doesn't recognize "--xhtml" parameter, it
>> recognizes only "-O xhtml" parameter.
>> 
>> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293
>
> Shouldn't the above be in commit message?

Actually, I prefer not having that "Reference" in the commit message
myself, but I do want to have some relevant details
missing from the proposed commit message but is in
that ticket (especially the comment #5) in the commit message.

> In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir.
> There is no --out-format either.

Thanks for digging about 2.4.5; in that case, unlike the redhat ticket
hinted, this change may not be backward compatible enough, as it seems
that the compatibility goes only back to 3.0.something.

>
> WTF this backward incompatibile change in highlight... the only
> solution that would make it work both for old and for new versions is
> to rely on the fact that HTML is default output format, i.e.
>
>   	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
>   	          quote_command($highlight_bin).
>  -	          " --xhtml --fragment --syntax $syntax |"
>  +	          " --fragment --syntax $syntax |"
>   		or die_error(500, "Couldn't open file or run syntax highlighter");

Adam, Jochen?  How does the counterproposal look to you?

Without knowing much about highlight nor using gitweb in general myself,
the above looks a bit more reasonable fix to the issue, if the default
format has been and will stay to be HTML.

^ permalink raw reply

* Re: Fwd: Git and Large Binaries: A Proposed Solution
From: Jakub Narebski @ 2011-01-26 21:40 UTC (permalink / raw)
  To: Scott Chacon; +Cc: Pete Wyckoff, Eric Montellese, Jeff King, git list
In-Reply-To: <AANLkTimE+s81Xbj4snNX0WWxG8x=qSwaQWfK+08+1Zy+@mail.gmail.com>

Scott Chacon <schacon@gmail.com> writes:

> Sorry to come in a bit late to this, but in addition to git-annex, I
> wrote something called 'git-media' a long time ago that works in a
> similar manner to what you both are discussing.
> 
> Much like what peff was talking about, it uses the smudge and clean
> filters to automatically redirect content into a .git/media directory
> instead of into Git itself while keeping the SHA in Git.  One of the
> cool thing is that it can use S3, scp or a local directory to transfer
> the big files to and from.
> 
> Check it out if interested:
> 
> https://github.com/schacon/git-media

Could you please add short information about this project to the
https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools
page, in the "Backups, metadata, and large files" subsection?
git-annex is there...

Thanks in advance.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [RFC] fast-import: 'cat-blob' and 'ls' commands
From: Jonathan Nieder @ 2011-01-26 21:39 UTC (permalink / raw)
  To: vcs-fast-import-devs
  Cc: David Barr, Git Mailing List, Ramkumar Ramachandra,
	Sverre Rabbelier, Shawn O. Pearce, Tomas Carnecky, Sam Vilain
In-Reply-To: <20110103080130.GA8842@burratino>

Hi fast importers,

I would like your thoughts on a few developments in fast-import
protocol (thanks to David, Ram, Sverre, Tomas, and Sam for work so
far).  If they seem good, I'd be happy to help make patches to other
backends so these can be implemented widely.

Contents: cat-blob command, filemodify (M) with trees, ls command.

cat-blob command
----------------

fast-import 1.7.4-rc0 added a new "cat-blob" feature.  It is meant to
allow exporters that receiving changes in delta form to avoid having
to remember the full text of blobs already exported or re-retrieve
them from the source repository.

It works like this:

1. Out of band, the fast-import frontend and backend negotiate a
   channel for the backend to send replies to the frontend.  In
   git fast-import, this is a file descriptor, defaulting to
   stdout.  So you can do:

	mkfifo replies &&

	$frontend <replies |
	git fast-import --cat-blob-fd=3 3>replies

   The intent is that stdin would typically be a socket and this file
   descriptor would point to that.

2. The frontend (optionally) declares use of this feature by putting

	feature cat-blob

   at the beginning of the stream.

3. When the frontend needs a previously exported blob to use as delta
   preimage, it uses the cat-blob command.

	cat-blob :3

   The backend replies with something like

	7c8987a987ca98c blob 6
	hello


   More precisely, the output format is

	<dataref> SP 'blob' SP <length> LF
	<full text of blob> LF

   The <dataref> can be any text not including whitespace.

   The frontend can rely on a little buffering if it wants to print a
   command after the "cat-blob", but it must read the reply in its
   entirety if it expects the backend to act on later commands.  In
   other words, the cat-blob command is not guaranteed to be
   asynchronous.

This protocol is used by the svn-fe[1] tool to handle Subversion dump
files in version 3 (--deltas) format and seems to work ok.

Does this look sane or does it need tweaking or more detailed
specification to be widely useful?  Even once git 1.7.4 is out, it
should be possible to make improvements using a new "feature" name.

filemodify (M) with trees
-------------------------

fast-import 1.7.3-rc0 introduced the ability for a filemodify (M)
command to place a tree named by mark or other <dataref> at a given
path, replacing whatever was there before.  The implementation had
some kinks, which fast-import 1.7.4-rc0 ironed out.

Without some way to specify marks or learn tree names out of band, it
is not very useful.  With some way to learn tree names, it can be
used, for example, to rewrite revision metadata while reusing the old
tree data:

	commit refs/heads/master
	mark :11
	committer A U Thor <author@example.com> Wed, 26 Jan 2011 15:14:11 -0600
	data <<EOF
	New change description
	EOF
	M 040000 4b825dc642cb6eb9a060e54bf8d69288fbee4904 ""

There is no "feature" name for this.  Corner case: a command to
replace a path with the empty tree is interpreted[2] as meaning to remove
that file or subtree, because git does not track empty directories.

Do the semantics seem reasonable?  Should this get a corresponding
"feature"?

ls command
----------

A patch in flight[3] introduces an "ls" command to read directory
entries from the active commit or a named commit.  This allows
printing a blob from the active commit or copying a blob or tree from
a previous commit for use in the current one.

It works like so:

1. Frontend writes

	'ls' SP <path> LF

or

	'ls' SP <dataref> SP <path> LF

  In the first form, the <path> _must_ be surrounded in quotes
  and quoted C-style.  In the second form, the <dataref> can refer
  to a tag, commit, or tree.

2. Backend replies through the cat-blob channel:

	<mode> SP <type> SP <dataref> HT <path> LF

   <mode> is a 6-digit octal mode: 040000, 100644, 100755,
   120000, or 160000 for a directory, regular file, executable file,
   symlink, or submodule, respectively.

   <type> is 'blob', 'tree', or 'commit'.

   <dataref> represents the corresponding blob, tree, or commit
   object.

   <path> is the path in question.  It can be quoted C-style and
   must be if the path starts with '"' or contains a newline.

3. Frontend reads the reply.  The frontend might use that <dataref> in
   a later filemodify (M) and cat-blob command.

Proposed updates to svn-fe[1] use this heavily and work well.

One ugly corner case: although it is intended to allow "missing
<path>" as a reply when the path is missing, the proposed patch
makes git fast-import use an empty tree to signal that case,
to ensure that, for example,

	ls ""
	M <mode> <dataref> ""

is always a non-operation.

No "feature" name yet.  Even better, it's not part of git yet so
I invite to nitpick to your heart's content.  Maybe you'd rather
the command be called "ls-tree" instead of "ls"?  Ask away. :)

Thoughts welcome, as always.
Jonathan

[1] http://repo.or.cz/w/git/jrn.git/blob/refs/heads/vcs-svn-pu:/vcs-svn/svndump.c
[2] Or rather, is not interpreted but ought to be, or else
fast-import will make it too easy to produce invalid commits.  One of
the patches in series [3] fixes it.
[3] http://thread.gmane.org/gmane.comp.version-control.git/162698/focus=164448

^ permalink raw reply

* Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"
From: Jakub Narebski @ 2011-01-26 21:34 UTC (permalink / raw)
  To: Adam Tkac; +Cc: git, gitster
In-Reply-To: <20110126171118.GA867@traged.englab.brq.redhat.com>

Adam Tkac <atkac@redhat.com> writes:

> Hello,
> 
> attached patch fixes the gitweb.perl script.

Please don't send patches as attachements, but inline.
See Documentation/SubmittingPatches

> Current highlight utility doesn't recognize "--xhtml" parameter, it
> recognizes only "-O xhtml" parameter.
> 
> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293

Shouldn't the above be in commit message?

> Subject: [PATCH] Pass "-O xhtml" parameter instead of "--xhtml" to hightlight in gitweb.

Why the mismatch between subject of email, and subject in attached
patch?

> Signed-off-by: Jochen Schmitt <jochen@herr-schmitt.de>
> Signed-off-by: Adam Tkac <atkac@redhat.com>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1025c2f..88556f4 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3468,7 +3468,7 @@ sub run_highlighter {
>  	close $fd;
>  	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
>  	          quote_command($highlight_bin).
> -	          " --xhtml --fragment --syntax $syntax |"
> +	          " -O xhtml --fragment --syntax $syntax |"
>  		or die_error(500, "Couldn't open file or run syntax highlighter");
>  	return $fd;
>  }

In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir.
There is no --out-format either.

WTF this backward incompatibile change in highlight... the only
solution that would make it work both for old and for new versions is
to rely on the fact that HTML is default output format, i.e.

  	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
  	          quote_command($highlight_bin).
 -	          " --xhtml --fragment --syntax $syntax |"
 +	          " --fragment --syntax $syntax |"
  		or die_error(500, "Couldn't open file or run syntax highlighter");


-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Francis Moreau @ 2011-01-26 21:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sverre Rabbelier, Johannes Sixt, git, René Scharfe
In-Reply-To: <7vei7zjr3y.fsf@alter.siamese.dyndns.org>

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

> Francis Moreau <francis.moro@gmail.com> writes:
>
>>> On Wed, Jan 26, 2011 at 21:56, Francis Moreau <francis.moro@gmail.com> wrote:
>>>> Just out of curiosity, I'd like to know why, since it has no annoyance
>>>> for those who are replying to my emails.
>>>
>>> We keep everybody on CC who's interested in the thread here, and
>>> usually reply directly to the person whose email we respond to. In
>>> this case, I had to manually edit the TO line to be just you, and move
>>> everybody else to the CC.
>>
>> Well, if I decided to set Mail-Followup-To, that do mean that I don't
>> want you to do that !
>
> That is _not_ something for _you_ to unilaterally decide.
>
> I am responding to _you_ with this message to tell _you_ something, while
> keeping Sverre, J6t and Rene and the list in the loop.
>
> If I followedyour M-F-T on the message I am responding to, I would have
> ended up placing these "secondary" (for the purpose of my message) on my
> "To:" line.  That would make it impossible for them to prioritize the
> messages addressed directly to them (which would have their address on
> "To:") over other messages that they are merely in the loop (which would
> not have them on "To:"---their address may be on "Cc:" or they may be
> getting the message from the list).
>
> So please don't.

Agreed.

Should be fixed now, if not, please let me know.

Thanks
-- 
Francis

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Francis Moreau @ 2011-01-26 21:31 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Johannes Sixt, git, René Scharfe
In-Reply-To: <AANLkTimzUFL0ByGcA1QRNL+br57ZY6OME4aFAT+ipaJV@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Wed, Jan 26, 2011 at 22:08, Francis Moreau <francis.moro@gmail.com> wrote:
>> Well, if I decided to set Mail-Followup-To, that do mean that I don't
>> want you to do that !
>
> Yes, but it makes it harder for us to do that. For example, now I had
> to remove myself from the to line, add you, and move everybody else to
> CC. In other words, please don't do it. Thanks :)

It should be fixed by now, please let me know it isn't !

I didn't know it works like this: I thought it was set so replies just
replace my email address with the mailing list one.

Thanks
-- 
Francis

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Junio C Hamano @ 2011-01-26 21:24 UTC (permalink / raw)
  To: Francis Moreau; +Cc: Sverre Rabbelier, Johannes Sixt, git, René Scharfe
In-Reply-To: <m2d3njz839.fsf@gmail.com>

Francis Moreau <francis.moro@gmail.com> writes:

>> On Wed, Jan 26, 2011 at 21:56, Francis Moreau <francis.moro@gmail.com> wrote:
>>> Just out of curiosity, I'd like to know why, since it has no annoyance
>>> for those who are replying to my emails.
>>
>> We keep everybody on CC who's interested in the thread here, and
>> usually reply directly to the person whose email we respond to. In
>> this case, I had to manually edit the TO line to be just you, and move
>> everybody else to the CC.
>
> Well, if I decided to set Mail-Followup-To, that do mean that I don't
> want you to do that !

That is _not_ something for _you_ to unilaterally decide.

I am responding to _you_ with this message to tell _you_ something, while
keeping Sverre, J6t and Rene and the list in the loop.

If I followedyour M-F-T on the message I am responding to, I would have
ended up placing these "secondary" (for the purpose of my message) on my
"To:" line.  That would make it impossible for them to prioritize the
messages addressed directly to them (which would have their address on
"To:") over other messages that they are merely in the loop (which would
not have them on "To:"---their address may be on "Cc:" or they may be
getting the message from the list).

So please don't.

Thanks.

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Sverre Rabbelier @ 2011-01-26 21:14 UTC (permalink / raw)
  To: Francis Moreau; +Cc: Johannes Sixt, git, René Scharfe
In-Reply-To: <m2d3njz839.fsf@gmail.com>

Heya,

On Wed, Jan 26, 2011 at 22:08, Francis Moreau <francis.moro@gmail.com> wrote:
> Well, if I decided to set Mail-Followup-To, that do mean that I don't
> want you to do that !

Yes, but it makes it harder for us to do that. For example, now I had
to remove myself from the to line, add you, and move everybody else to
CC. In other words, please don't do it. Thanks :)

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Francis Moreau @ 2011-01-26 21:08 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Johannes Sixt, git, René Scharfe
In-Reply-To: <AANLkTinKZfkK0uuyNJZf7f1hvfu5-LeSsxaE9fkQ2O5X@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Wed, Jan 26, 2011 at 21:56, Francis Moreau <francis.moro@gmail.com> wrote:
>> Just out of curiosity, I'd like to know why, since it has no annoyance
>> for those who are replying to my emails.
>
> We keep everybody on CC who's interested in the thread here, and
> usually reply directly to the person whose email we respond to. In
> this case, I had to manually edit the TO line to be just you, and move
> everybody else to the CC.

Well, if I decided to set Mail-Followup-To, that do mean that I don't
want you to do that !

-- 
Francis

^ permalink raw reply

* Re: Can't find the revelant commit with git-log
From: Sverre Rabbelier @ 2011-01-26 21:03 UTC (permalink / raw)
  To: francis.moro; +Cc: Johannes Sixt, git, René Scharfe
In-Reply-To: <m2hbcvz8me.fsf@gmail.com>

Heya,

On Wed, Jan 26, 2011 at 21:56, Francis Moreau <francis.moro@gmail.com> wrote:
> Just out of curiosity, I'd like to know why, since it has no annoyance
> for those who are replying to my emails.

We keep everybody on CC who's interested in the thread here, and
usually reply directly to the person whose email we respond to. In
this case, I had to manually edit the TO line to be just you, and move
everybody else to the CC.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] fsck: do not give up too early in fsck_dir()
From: Junio C Hamano @ 2011-01-26 21:01 UTC (permalink / raw)
  To: git

When there is a random garbage file whose name happens to be 38-byte
long in a .git/objects/??/ directory, the loop terminated prematurely
without marking all the other files that it hasn't checked in the
readdir() loop.

Treat such a file just like any other garbage file, and do not break out
of the readdir() loop.

While at it, replace repeated sprintf() calls to a single one outside the
loop.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * For readability, I think it also makes sense to move the surrounding
   code so that "tmp_obj_" check and the "is_dot_or_dotdot" check are next
   to each other, as they are both in "we know this is not a loose object
   file, and we don't want to warn about its existence" category, but that
   is a minor point.

 builtin/fsck.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 91409a0..795aba0 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -392,10 +392,20 @@ static void add_sha1_list(unsigned char *sha1, unsigned long ino)
 	sha1_list.nr = ++nr;
 }
 
+static inline int is_loose_object_file(struct dirent *de,
+				       char *name, unsigned char *sha1)
+{
+	if (strlen(de->d_name) != 38)
+		return 0;
+	memcpy(name + 2, de->d_name, 39);
+	return !get_sha1_hex(name, sha1);
+}
+
 static void fsck_dir(int i, char *path)
 {
 	DIR *dir = opendir(path);
 	struct dirent *de;
+	char name[100];
 
 	if (!dir)
 		return;
@@ -403,17 +413,13 @@ static void fsck_dir(int i, char *path)
 	if (verbose)
 		fprintf(stderr, "Checking directory %s\n", path);
 
+	sprintf(name, "%02x", i);
 	while ((de = readdir(dir)) != NULL) {
-		char name[100];
 		unsigned char sha1[20];
 
 		if (is_dot_or_dotdot(de->d_name))
 			continue;
-		if (strlen(de->d_name) == 38) {
-			sprintf(name, "%02x", i);
-			memcpy(name+2, de->d_name, 39);
-			if (get_sha1_hex(name, sha1) < 0)
-				break;
+		if (is_loose_object_file(de, name, sha1)) {
 			add_sha1_list(sha1, DIRENT_SORT_HINT(de));
 			continue;
 		}

^ permalink raw reply related

* Re: Can't find the revelant commit with git-log
From: Francis Moreau @ 2011-01-26 20:56 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, René Scharfe
In-Reply-To: <4D3FFB0F.9070700@viscovery.net>

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

> Please don't set Mail-Followup-To; it's disliked on this list.

Just out of curiosity, I'd like to know why, since it has no annoyance
for those who are replying to my emails.

>
>
> Am 1/26/2011 9:36, schrieb Francis Moreau:
>> I tried to reproduce something similar but with a far more simple repo:
>> 
>> 
>> <v2.6.28> 1.f o
>>               |
>>           1.e o (merge)
>>               | \
>>           1.d o  o 2.c (merge)
>>               |  | \
>>           1.c o  o  o 3.a "Remove blacklist_iommu()"
>>               |  | /
>>               |  o 2.a
>>               | /
>>           1.b o
>>               |
>> <v2.6.27> 1.a o "Introduce blacklist_iommu()"
>>               |
>>               o Init
>> 
>> Basically this repo 3 branches: master, 2, 3. Master branch introduces
>> the "blacklist_iommu()" function with commit 1.a, and branch "3" removes
>> it at commit 3.a.
>> ...
>> So in this case there's no need to pass the '-m' flag and git-log(1), by
>> default walks through all the commits:
>
> To reproduce the real history, you have to modify your example in three ways:
>
> 1. 2.a must be forked off of Init, not 1.b; i.e., this commit does not
> contain "blacklist_iommu".
>
> 2. Drop the side branch that removes the word. (Drop at least the commit.)
>
> 3. The merge 1.e (which resembles d847059) must be modified such that it
> takes the contents of 2.c rather than 1.d.
>
> IOW, "blacklist_iommu" is not removed explicitly by a commit, but rather
> by a merge of one branch that has it and another one that doesn't have it.
>
> Look closely at d847059: The commit message hints at a conflict in
> intel_iommu.c,

But how did you find out d847059 ?

Did you use René's method ?

Thanks
-- 
Francis

^ permalink raw reply

* [PATCH] fsck: drop unused parameter from traverse_one_object()
From: Junio C Hamano @ 2011-01-26 20:46 UTC (permalink / raw)
  To: git

Also add comments to seemingly unsafe pointer dereferences, that
are all safe.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fsck.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6d5ebca..91409a0 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -74,7 +74,13 @@ static int mark_object(struct object *obj, int type, void *data)
 {
 	struct object *parent = data;
 
+	/*
+	 * The only case data is NULL or type is OBJ_ANY is when
+	 * mark_object_reachable() calls us.  All the callers of
+	 * that function has non-NULL obj hence ...
+	 */
 	if (!obj) {
+		/* ... these references to parent->fld are safe here */
 		printf("broken link from %7s %s\n",
 			   typename(parent->type), sha1_to_hex(parent->sha1));
 		printf("broken link from %7s %s\n",
@@ -84,6 +90,7 @@ static int mark_object(struct object *obj, int type, void *data)
 	}
 
 	if (type != OBJ_ANY && obj->type != type)
+		/* ... and the reference to parent is safe here */
 		objerror(parent, "wrong object type in link");
 
 	if (obj->flags & REACHABLE)
@@ -109,7 +116,7 @@ static void mark_object_reachable(struct object *obj)
 	mark_object(obj, OBJ_ANY, NULL);
 }
 
-static int traverse_one_object(struct object *obj, struct object *parent)
+static int traverse_one_object(struct object *obj)
 {
 	int result;
 	struct tree *tree = NULL;
@@ -138,7 +145,7 @@ static int traverse_reachable(void)
 		entry = pending.objects + --pending.nr;
 		obj = entry->item;
 		parent = (struct object *) entry->name;
-		result |= traverse_one_object(obj, parent);
+		result |= traverse_one_object(obj);
 	}
 	return !!result;
 }
@@ -556,8 +563,8 @@ static int fsck_cache_tree(struct cache_tree *it)
 			      sha1_to_hex(it->sha1));
 			return 1;
 		}
-		mark_object_reachable(obj);
 		obj->used = 1;
+		mark_object_reachable(obj);
 		if (obj->type != OBJ_TREE)
 			err |= objerror(obj, "non-tree in cache-tree");
 	}
-- 
1.7.4.rc3.259.g12451

^ permalink raw reply related

* Re: Updating a submodule with a compatible version from another submodule version using the parent meta-repository
From: Julian Ibarz @ 2011-01-26 20:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, git
In-Reply-To: <7v7hdrl7nw.fsf@alter.siamese.dyndns.org>

On Wed, Jan 26, 2011 at 3:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Julian Ibarz <julian.ibarz@gmail.com> writes:
>
>>> Hmm, looks like I lost you here ... you want to bisect in B although
>>> you know what commit you want there? Care to explain a bit more?
>>
>> In B I have a feature to integrate in master branch. This feature is
>> in branch old_feature. But this branch is really old. To try this
>> feature I need to rebuild it at this version. To make the build
>> success I need also to revert back the submodule C because B is
>> dependent on it. But finding the good version of C that match
>> old_feature version is a pain... Is it clear?
>
> That sounds like bisecting in C with a frozen checkout of B to see which
> version in C works well with that target commit in B you know you want to
> use.  Why do you need to bisect in B???
>

Forget about bisect. This is a different use case where I need the
feature I am talking about: checkout an old version in B and
automatically having A and C switch to a compatible version (the cause
can be because of a bisect or just because I want to try an old
feature not yet integrated into master or whatever the reason I want
to be on this old version).

^ permalink raw reply

* Re: Updating a submodule with a compatible version from another submodule version using the parent meta-repository
From: Julian Ibarz @ 2011-01-26 20:43 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git
In-Reply-To: <4D40849C.2050903@web.de>

> Am 26.01.2011 20:48, schrieb Julian Ibarz:
>> Basically my feature would work like this:
>>
>> in B:
>> git submodule checkout some_version
>>
>> This will checkout B but also change A and C so that it is compatible
>> with some_version of B. Basically it will find the commit in A that
>> has the closest parent commit of some_version in B. When this is done
>> it just does git submodule udate on other submodules.
>
> Thanks, now I understand what you are trying to achieve.
>
>> I see in gitk that there is a feature that has a common implementation
>> for what I want to do:
>>
>> For every commits you can see Follows and Precedes which lists the
>> closest label before this release and after. What I need is the same
>> thing: instead of finding a closest labeled commit, I need to find a
>> closest commit referenced by A that precedes current HEAD of B. When
>> this is done I know which commit A has to be and then just have to
>> call git submodule update in A (update every other submodules except
>> for B).
>
> I am not aware of something like that in current Git, But I see that
> such functionality would be helpful. Care to share your implementation
> idea?

Well actually the paragraph just above is what is my implementation
idea. I recognize it is really high level but it is still a start ;).
Basically the "complex" part is to find the precedent commit in B that
is referenced by A. Since gitk has this already but for labels, we
just need to reuse/copy their function that does this but instead as
input having a list of commit labels and find the closest precedent it
is instead the list of commits referenced by A. By the way, anyone
reading this knows where it is in the code of gitk? Or if there is
something like that already in the git code? I never looked into both
code projects.

^ permalink raw reply

* Re: [PATCH] tests: sanitize more git environment variables
From: Junio C Hamano @ 2011-01-26 20:43 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110126203331.GA27478@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> These variables should generally not be set in one's
> environment, but they do get set by rebase, which means
> doing an interactive rebase like:
>
>   pick abcd1234 foo
>   exec make test
>
> will cause false negatives in the test suite.

Cute.  Thanks.

^ permalink raw reply

* Re: Updating a submodule with a compatible version from another submodule version using the parent meta-repository
From: Junio C Hamano @ 2011-01-26 20:41 UTC (permalink / raw)
  To: Julian Ibarz; +Cc: Jens Lehmann, git
In-Reply-To: <AANLkTik-XdgGM20kFu8KZ5k9ynfNAo8fvL9t7kL_JhQg@mail.gmail.com>

Julian Ibarz <julian.ibarz@gmail.com> writes:

>> Hmm, looks like I lost you here ... you want to bisect in B although
>> you know what commit you want there? Care to explain a bit more?
>
> In B I have a feature to integrate in master branch. This feature is
> in branch old_feature. But this branch is really old. To try this
> feature I need to rebuild it at this version. To make the build
> success I need also to revert back the submodule C because B is
> dependent on it. But finding the good version of C that match
> old_feature version is a pain... Is it clear?

That sounds like bisecting in C with a frozen checkout of B to see which
version in C works well with that target commit in B you know you want to
use.  Why do you need to bisect in B???

^ permalink raw reply

* Re: [PATCH] tests: sanitize more git environment variables
From: Jeff King @ 2011-01-26 20:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20110126203331.GA27478@sigill.intra.peff.net>

On Wed, Jan 26, 2011 at 03:33:32PM -0500, Jeff King wrote:

> These variables should generally not be set in one's
> environment, but they do get set by rebase, which means
> doing an interactive rebase like:
> 
>   pick abcd1234 foo
>   exec make test
> 
> will cause false negatives in the test suite.

Or I guess false positives, depending on your definition of positive and
negative. I.e., tests will fail when they shouldn't. :)

-Peff

^ permalink raw reply

* [PATCH] tests: sanitize more git environment variables
From: Jeff King @ 2011-01-26 20:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

These variables should generally not be set in one's
environment, but they do get set by rebase, which means
doing an interactive rebase like:

  pick abcd1234 foo
  exec make test

will cause false negatives in the test suite.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 42f2f14..0fdc541 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -70,6 +70,9 @@ unset GIT_NOTES_REF
 unset GIT_NOTES_DISPLAY_REF
 unset GIT_NOTES_REWRITE_REF
 unset GIT_NOTES_REWRITE_MODE
+unset GIT_REFLOG_ACTION
+unset GIT_CHERRY_PICK_HELP
+unset GIT_QUIET
 GIT_MERGE_VERBOSITY=5
 export GIT_MERGE_VERBOSITY
 export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
-- 
1.7.0.9.7.g36b59

^ permalink raw reply related


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