Git development
 help / color / mirror / Atom feed
* [PATCH] Show a failure of rebase -p if the merge had a conflict
From: Johannes Sixt @ 2008-12-11 16:21 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Andreas Ericsson, Stephen Haberman, git, Junio C Hamano,
	Johannes Sixt

This extends t3409-rebase-preserve-merges by a case where the merge that
is rebased has a conflict. Therefore, the rebase stops and expects that
the user resolves the conflict. However, currently rebase --continue
fails because .git/rebase-merge/author-script is missing.

The test script had allocated two identical clones, but only one of them
(clone2) was used. Now we use both as indicated in the comment. Also, an
instance of && was missing in the setup part.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  BTW, I'm not 100% sure whether the additional tests of what to expect
  from the test if it did not fail are correct.

  I am unable to fix the failure.

  -- Hannes

  [Sorry, Junio, for the resend. git send-email & PEBCAK. :-/ ]

 t/t3409-rebase-preserve-merges.sh |   43 ++++++++++++++++++++++++++++++++----
 1 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 8cde40f..02a6401 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -11,7 +11,7 @@ Run "git rebase -p" and check that merges are properly carried along
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
-#echo 'Setting up:
+#Clone 1 (trivial merge):
 #
 #A1--A2  <-- origin/master
 # \   \
@@ -19,7 +19,15 @@ export GIT_AUTHOR_EMAIL
 #   \
 #    B2  <-- origin/topic
 #
-#'
+#Clone 2 (conflicting merge):
+#
+#A1--A2--B3   <-- origin/master
+# \       \
+#  B1------M  <-- topic
+#   \
+#    B2       <-- origin/topic
+#
+# In both cases, 'topic' is rebased onto 'origin/topic'.
 
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
@@ -37,12 +45,19 @@ test_expect_success 'setup for merge-preserving rebase' \
 	cd clone1 &&
 	git checkout -b topic origin/topic &&
 	git merge origin/master &&
-	cd ..
+	cd .. &&
+
+	echo Fifth > B &&
+	git add B &&
+	git commit -m "Add different B" &&
 
 	git clone ./. clone2
 	cd clone2 &&
 	git checkout -b topic origin/topic &&
-	git merge origin/master &&
+	test_must_fail git merge origin/master &&
+	echo Resolved > B &&
+	git add B &&
+	git commit -m "Merge origin/master into topic" &&
 	cd .. &&
 
 	git checkout topic &&
@@ -51,11 +66,29 @@ test_expect_success 'setup for merge-preserving rebase' \
 '
 
 test_expect_success 'rebase -p fakes interactive rebase' '
-	cd clone2 &&
+	(
+	cd clone1 &&
 	git fetch &&
 	git rebase -p origin/topic &&
 	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
 	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+	)
+'
+
+test_expect_failure '--continue works after a conflict' '
+	(
+	cd clone2 &&
+	git fetch &&
+	test_must_fail git rebase -p origin/topic &&
+	test 2 = $(git ls-files B | wc -l) &&
+	echo Resolved again > B &&
+	test_must_fail git rebase --continue &&
+	git add B &&
+	git rebase --continue &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Add different" | wc -l) &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge origin" | wc -l)
+	)
 '
 
 test_done
-- 
1.6.1.rc2.22.gf3bf84

^ permalink raw reply related

* Re: fatal output from git-show really wants a terminal
From: Boyd Stephen Smith Jr. @ 2008-12-11 16:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0812111015140.18321@eeepc-johanness>

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

On Thursday 2008 December 11 03:15:47 you wrote:
>On Wed, 10 Dec 2008, Boyd Stephen Smith Jr. wrote:
>> On Wednesday 2008 December 10 13:46:50 you wrote:
>> >On Mittwoch, 10. Dezember 2008, Tim Olsen wrote:
>> >> It appears that when outputting a fatal error, git-show will choose
>> >> stdout over stderr if stdout is a terminal and stderr is not.
>> >
>> >This is by design.
>>
>> Then it is poor design. :P j/k
>
>Read up on the reasoning before trolling, will ya?  It's all in the Git
>history.

Seeing how I'm new, and this message indicated I had screwed up, I starting 
going through the 'git log' looking for a commit message that either 
documented this behavior, or indicated the commit had documented this 
behavior.

Initially, I was looking for 'stdout' or 'stderr', and found many unrelated 
commits.  I then figured it was part of the PAGER support, and began 
searching for that.  I did find an indication of why stdout and stderr are 
both redirected to the PAGER's stdin -- but that makes sense to me; I wasn't 
questioning it.  At least not too much -- but when the user indicates stderr 
and stdout should go to different locations, shouldn't they?

I was mainly questioning using a pager AT ALL when the git command is used in 
a non-interactive environment, and how git detects an interactive invocation.  
I feel this should be done the same way a (POSIX standard) shell detects 
interactivity, and that in a non-interactive environment git should not 
default to using PAGER.

Now, I certainly could have missed the commit message / commit with 
rationale / documentation.  'git log' output is a long document, and I maybe 
using the wrong keywords for my search.  It also is not all the documentation 
that is out there.  I'm not afraid to RTFM; but I'm not having much luck 
finding the right parts to R.

Finally, I didn't mean to offend.  I was hoping the smiley (":P") and "j/k" 
would indicate that a was only half serious and know that I don't have the 
benefit of following the project closely for very long.  I'm appreciative of 
the hard work that goes into git and don't mean to belittle that effort.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss03@volumehost.net                      ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.org/                      \_/     

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

^ permalink raw reply

* [JGIT PATCH 2/5 v2] Add copy(InputStream) to TemporaryBuffer
From: Shawn O. Pearce @ 2008-12-11 16:53 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <20081211155218.GF32487@spearce.org>

In some places we may find ourselves with an InputStream we
need to copy into a TemporaryBuffer, so we can flatten out the
entire stream to a single byte[].  Putting the copy loop here
is more useful then duplicating it in application level code.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
  "Shawn O. Pearce" <spearce@spearce.org> wrote:
  > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
  > > torsdag 11 december 2008 05:58:39 skrev Shawn O. Pearce:
  > > > +		final byte[] b = new byte[2048];
  > >
  > > Why not 8192 here too?
  > 
  > Blargh, you're right.  Actually what I should do is look to see
  > if blocks != null, in which case I should alloc a block and read
  > directly into it.  That would avoid one copy of the data.

  And now we do that...
  
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
index b1ffd6e..761f359 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
@@ -42,6 +42,7 @@
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
 
@@ -135,6 +136,39 @@ public void write(final byte[] b, int off, int len) throws IOException {
 			diskOut.write(b, off, len);
 	}
 
+	/**
+	 * Copy all bytes remaining on the input stream into this buffer.
+	 * 
+	 * @param in
+	 *            the stream to read from, until EOF is reached.
+	 * @throws IOException
+	 *             an error occurred reading from the input stream, or while
+	 *             writing to a local temporary file.
+	 */
+	public void copy(final InputStream in) throws IOException {
+		if (blocks != null) {
+			for (;;) {
+				Block s = last();
+				if (s.isFull()) {
+					if (reachedInCoreLimit())
+						break;
+					s = new Block();
+					blocks.add(s);
+				}
+
+				final int n = in.read(s.buffer, s.count, Block.SZ - s.count);
+				if (n < 1)
+					return;
+				s.count += n;
+			}
+		}
+
+		final byte[] tmp = new byte[Block.SZ];
+		int n;
+		while ((n = in.read(tmp)) > 0)
+			diskOut.write(tmp, 0, n);
+	}
+
 	private Block last() {
 		return blocks.get(blocks.size() - 1);
 	}
-- 
1.6.1.rc2.306.ge5d5e


-- 
Shawn.

^ permalink raw reply related

* Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: rdkrsr @ 2008-12-11 17:15 UTC (permalink / raw)
  To: git
In-Reply-To: <d304880b0812110142g41b80745ic09a7200e02dcdb0@mail.gmail.com>

I'm sorry that I didn't answer to git mailing list address. So here
comes the email again.


Here is what I did:

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/li
t linux-2.6
Initialize linux-2.6/.git
Initialized empty Git repository in c:/Dokumente und Einstellungen/Ge
Eigene Dateien/vbox/linux-2.6/.git/
remote: Counting objects: 980697, done.←[K
remote: Compressing objects: 100% (161545/161545), done.←[K
remote: Total 980697 (delta 818552), reused 978923 (delta 816954)←[K
Receiving objects: 100% (980697/980697), 236.14 MiB | 90 KiB/s, done.
Resolving deltas: 100% (818552/818552), done.
Checking out files: 100% (25254/25254), done.

$ cd linux-2.6

$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   Documentation/IO-mapping.txt
#       modified:   include/linux/netfilter/xt_CONNMARK.h
#       modified:   include/linux/netfilter/xt_DSCP.h
#       modified:   include/linux/netfilter/xt_MARK.h
#       modified:   include/linux/netfilter/xt_RATEEST.h
#       modified:   include/linux/netfilter/xt_TCPMSS.h
#       modified:   include/linux/netfilter_ipv4/ipt_CONNMARK.h
#       modified:   include/linux/netfilter_ipv4/ipt_DSCP.h
#       modified:   include/linux/netfilter_ipv4/ipt_ECN.h
#       modified:   include/linux/netfilter_ipv4/ipt_MARK.h
#       modified:   include/linux/netfilter_ipv4/ipt_TCPMSS.h
#       modified:   include/linux/netfilter_ipv4/ipt_TOS.h
#       modified:   include/linux/netfilter_ipv4/ipt_TTL.h
#       modified:   include/linux/netfilter_ipv6/ip6t_HL.h
#       modified:   include/linux/netfilter_ipv6/ip6t_MARK.h
#       modified:   net/ipv4/netfilter/ipt_ECN.c
#       modified:   net/ipv4/netfilter/ipt_TTL.c
#       modified:   net/ipv6/netfilter/ip6t_HL.c
#       modified:   net/netfilter/xt_CONNMARK.c
#       modified:   net/netfilter/xt_DSCP.c
#       modified:   net/netfilter/xt_MARK.c
#       modified:   net/netfilter/xt_RATEEST.c
#       modified:   net/netfilter/xt_TCPMSS.c
#
no changes added to commit (use "git add" and/or "git commit -a")

2008/12/10 Brett Simmers <swtaarrs@gmail.com>:
> On Wed, Dec 10, 2008 at 1:22 PM, rdkrsr <rdkrsr@googlemail.com> wrote:
>> I just fetched the sources without changing anything, but git diff
>> shows, that there are changes that are not yet updated (changed but not
>> updated: use git add to ...). Why is it like that?
>>
>> I use msysgit on windows, maybe that is one reason?
>
> What are the filenames? I've seen git on Windows get confused if a
> repository has two files that are the same except for the case of some
> of the letters (since both can't exist by default on NTFS).
>
> -Brett
>

^ permalink raw reply

* Re: epic fsck SIGSEGV!
From: Linus Torvalds @ 2008-12-11 17:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, R. Tyler Ballance, Johannes Sixt, Git Mailing List
In-Reply-To: <7v63lrupxk.fsf@gitster.siamese.dyndns.org>



On Wed, 10 Dec 2008, Junio C Hamano wrote:
> 
> I'll consider this signed-off and do the usual forging

Yea. I've even tested it a bit now:

	[torvalds@nehalem git]$ ulimit -s 1024
	[torvalds@nehalem git]$ git fsck --full
	Segmentation fault
	[torvalds@nehalem git]$ ./git-fsck --full
	dangling commit 3d00b49495ceff119de52dc5443731e2d8d84b6b
	dangling commit 4e0a3c7de9af3cbb53cc421329f0579679edbb51
	...

so it does seem to fix the issue, and the patch looks safe enough.

It passes all the tests, and works fine on the kernel repo too (ugh, four 
minutes! I used to run git-fsck religiously every day back in the early 
days, now I realized that I must not have done so in _months_, and my 
kernel tree has grown and so has fsck time).

But obviously the true test for fsck is some complex corruption, and I 
didn't test that. I can't imagine that it introduces any new problems 
though - but the bugs you can't imagine are always the worst ones ;)

			Linus

^ permalink raw reply

* Re: [RFC/PATCH 4/3] gitweb: Incremental blame (proof of concept)
From: Jakub Narebski @ 2008-12-11 17:28 UTC (permalink / raw)
  To: git; +Cc: Petr Baudis, Fredrik Kuivinen
In-Reply-To: <20081210200908.16899.36727.stgit@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> This is tweaked up version of Petr Baudis <pasky@suse.cz> patch, which
> in turn was tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s
> proof of concept patch.  It adds 'blame_incremental' view, which
> incrementally displays line data in blame view using JavaScript (AJAX).

[...]
> Patch by Petr Baudis this one is based on:
>   http://permalink.gmane.org/gmane.comp.version-control.git/56657
> 
> Original patch by Fredrik Kuivinen:
>   http://article.gmane.org/gmane.comp.version-control.git/41361
> 
> Snippet adding 'generated in' to gitweb, by Petr Baudis:
>   http://article.gmane.org/gmane.comp.version-control.git/83306
> 
> Should I post interdiff to Petr Baudis patch, and comments about
> difference between them? [...]

Here is the list of differences between Petr Baudis patch and the one
I have just send.  No interdiff, as it is artificially large because
previous patch was based on much older version, so ranges does not
match.

Bugs I have made:
 * I forgot to make some changes for git-instaweb.sh to have support
   for incremental blame, namely dependency of 'git-instaweb' target
   in Makefile on gitweb/blame.js, and lack of the following line in
   git-instaweb.sh:
        gitweb_blamejs $GIT_DIR/gitweb/blame.js

 * Pasky's patch added support for href(...,-partial_query=>1) extra
   parameter, which ensured that gitweb link had '?' in it, and used
   it to generate 'baseUrl' parameter for startBlame.  I have
   misunderstood what baseUrl is about, and used $my_url there, while
   it is partial URL for blame links: it is projectUrl.

   Therefore links in blame table current 'blame_incremental' would
   not work. I'm sorry about that, I thought I have checked it...

Intentionally omitted features:
 * In patch this one is based on there was fixBlameLinks() JavaScript
   function (put directly in the HTML head inside <script> element),
   which was used in body.onLoad event to change 'a=blame' to
   'a=blame_incremental' in links marked with class="blamelink".

   First, this IMHO should be put as separate patch; you can test
   'blame_incremental' view by hand-crafting gitweb URL.  Second, it
   would be not enough in current gitweb, as action can be put in
   path_info. So either fixBlameLinks() should be made work in both
   cases, or it should be done in different way, for example adding
   'js=1' for all links, or doing JavaScript redirect from 'blame'
   view (although this way we won't be able to view ordinary 'blame'
   view without turning off JavaScript).

Differences in coding of the same features:
 * In Pasky's patch git_blame (then named git_blame2) and
   git_blame_incremental were just wrappers around git_blame_common;
   in this patch git_blame_data is also wrapper (to avoid duplicating
   permissions and parameter checking code).

 * Instead of calculating title string for "Commit" column cell, and
   necessary data for each row, we now calculate it once per commit
   and save (cache) in 'commits' associative array (hash).

   This is a bit of performance improvement, similar to the one in 
     "[PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()"
   for 'blame' view in gitweb. It is just using Date() and string
   concatenation, and not extra fork.

 * Variables holding manipulated elements are named a bit differently,
   and calculated upfront:
      td_sha1   instead of  tr.firstChild
      a_sha1    instead of  ahsAnchor
      a_linenr  instead of  lineAnchor

 * There were a few of style changes in gitweb/blame.js; for example
   it is used the following style of function definition

      function functionName(arg1, arg2) {

   thorough the code.

Fixes for bugs in Pasky's patch, and changes related to changes in
ordinary 'blame' output: 
 * handleResponse function was called only from XMLHttpRequest
   onreadystatechange event. Not all browsers call onreadystatechange
   each time the server flushes output (Firefox does that), so we use
   a timer to check (poll) for changes.

   See http://ajaxpatterns.org/HTTP_Streaming#Refactoring_Illustration

 * The 'linenr' link was to line number commit.srcline, while it
   should be to (commit.srcline + i), as commit.srcline is _start_
   line in group, and not the line equivalent to current line in
   source.

   This might be thought a (mis)feature, and not a bug, though.

 * Currently 'blame' view uses single cell (with rowspan="N") spanning
   the whole group of lines blaming the same commit, instead of using
   empty cells for subsequent lines in group.  Therefore instead of
   setting "shaAnchor.innerHTML = '';" to make subsequent cells in
   "Commit" ('sha1') column empty (or rather to make link element <a>
   in cell empty), we use "tr.removeChild(td_sha1);"

   This change was made necessary by changes in the 'blame' view.
   This also meant that the code checking if lines are in the same
   group has to be changes; it was refactored into startOfGroup(tr)
   function.

 * The title for cells in "Commit" column used UTC (GMT) date and time
      'Kay Sievers, 2005-08-07 19:49:46'
   instead of the localtime format used currently by 'blame' view:
      'Kay Sievers, 2005-08-07 21:49:46 +0200'
   Current code uses neither, but 'commit'-like format:
      'Kay Sievers, 2005-08-07 19:49:46 +0000 (21:49:46 +0200)'

New features (in short):
 * 3-coloring of lines with blame data during incremental blaming
 * Adding author initials below shortened SHA-1 of a commit
   (if there is place for it, i.e. if group has more than 1 row)
 * progress indicator: progress info and progress bar
 * information about how long it took to run 'blame_data',
   and how long it took to run JavaScript script

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: Linus Torvalds @ 2008-12-11 17:41 UTC (permalink / raw)
  To: rdkrsr; +Cc: git
In-Reply-To: <d304880b0812110915o6968050cufbb1e29c8bcea984@mail.gmail.com>



On Thu, 11 Dec 2008, rdkrsr wrote:
>
> I'm sorry that I didn't answer to git mailing list address. So here
> comes the email again.

You have a broken filesystem.

> $ git status
> # On branch master
> # Changed but not updated:
> #   (use "git add <file>..." to update what will be committed)
> #
> #       modified:   Documentation/IO-mapping.txt
> #       modified:   include/linux/netfilter/xt_CONNMARK.h
> #       modified:   include/linux/netfilter/xt_DSCP.h
> #       modified:   include/linux/netfilter/xt_MARK.h
> #       modified:   include/linux/netfilter/xt_RATEEST.h
...

This is _exactly_ what happens if you try to develop the Linux kernel on a 
case-insensitive filesystem. The kernel source tree has several files that 
differ only in case, eg

	Documentation/IO-mapping.txt
	Documentation/io-mapping.txt
	include/linux/netfilter/xt_tcpmss.h
	include/linux/netfilter/xt_TCPMSS.h
	..

and if you try to check it out on a broken filesystem, then the second 
file will overwrite the first one, and git will think that you have 
modified it. 

OS X? Afaik, you can fix it by using NFS or UFS. And I think ZFS has a 
case-sensitive mode too (and it may even be the default). In fact, I think 
newer versions of OS X even allow that piece-of-sh*t HFS+ to be case 
sensitive (and thus make it much less sh*tty).

Of course, there are reports of some Mac software breaking when they use a 
real filesystem, but hey, what else is new?

			Linus

^ permalink raw reply

* Re: Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: rdkrsr @ 2008-12-11 17:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0812110934180.3340@localhost.localdomain>

Thank you, Linus and Brett, for your answers.

I'm not developing linux kernel, I just wanted to experiment with git.
And then I didn't know if this is a normal behaviour of git. I'm using
windows xp and msysgit for this. And the file system is NTFS. I'm
using dual boot to sporadicly use linux and tried also linux in
virtual box. But both isn't really good. Maybe one day I dare to use
linux as my primary OS.

Red

2008/12/11 Linus Torvalds <torvalds@linux-foundation.org>:
>
>
> On Thu, 11 Dec 2008, rdkrsr wrote:
>>
>> I'm sorry that I didn't answer to git mailing list address. So here
>> comes the email again.
>
> You have a broken filesystem.
>
>> $ git status
>> # On branch master
>> # Changed but not updated:
>> #   (use "git add <file>..." to update what will be committed)
>> #
>> #       modified:   Documentation/IO-mapping.txt
>> #       modified:   include/linux/netfilter/xt_CONNMARK.h
>> #       modified:   include/linux/netfilter/xt_DSCP.h
>> #       modified:   include/linux/netfilter/xt_MARK.h
>> #       modified:   include/linux/netfilter/xt_RATEEST.h
> ...
>
> This is _exactly_ what happens if you try to develop the Linux kernel on a
> case-insensitive filesystem. The kernel source tree has several files that
> differ only in case, eg
>
>        Documentation/IO-mapping.txt
>        Documentation/io-mapping.txt
>        include/linux/netfilter/xt_tcpmss.h
>        include/linux/netfilter/xt_TCPMSS.h
>        ..
>
> and if you try to check it out on a broken filesystem, then the second
> file will overwrite the first one, and git will think that you have
> modified it.
>
> OS X? Afaik, you can fix it by using NFS or UFS. And I think ZFS has a
> case-sensitive mode too (and it may even be the default). In fact, I think
> newer versions of OS X even allow that piece-of-sh*t HFS+ to be case
> sensitive (and thus make it much less sh*tty).
>
> Of course, there are reports of some Mac software breaking when they use a
> real filesystem, but hey, what else is new?
>
>                        Linus
>

^ permalink raw reply

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Robin Rosenberg @ 2008-12-11 18:34 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1228971522-28764-5-git-send-email-spearce@spearce.org>

torsdag 11 december 2008 05:58:41 skrev Shawn O. Pearce:
> Most patch scripts impact more than one file at a time, so we need
> to support parsing multiple FileHeaders from the same input stream
> and collect them into a larger entity representing the entire script.
...
> +	public void testParse_ConfigCaseInsensitive() throws IOException {
> +		final Patch p = parseTestPatchFile();
> +		assertEquals(2, p.getFiles().size());
> +
> +		final FileHeader fRepositoryConfigTest = p.getFiles().get(0);
> +		final FileHeader fRepositoryConfig = p.getFiles().get(1);
> +
> +		assertEquals(
> +				"org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java",
> +				fRepositoryConfigTest.getNewName());
> +
> +		assertEquals(
> +				"org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java",
> +				fRepositoryConfig.getNewName());
> +
> +		assertEquals(572, fRepositoryConfigTest.startOffset);
> +		assertEquals(1490, fRepositoryConfig.startOffset);
1487 here

-- robin

^ permalink raw reply

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Shawn O. Pearce @ 2008-12-11 18:39 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200812111934.13218.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > +		assertEquals(572, fRepositoryConfigTest.startOffset);
> > +		assertEquals(1490, fRepositoryConfig.startOffset);
>
> 1487 here

Really?  1490 is the only value that the test vector passes with.
What's the 3 bytes you think I'm off by?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v2] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Junio C Hamano @ 2008-12-11 19:26 UTC (permalink / raw)
  To: Fabian Franz; +Cc: FabianFranz, git, hjemli
In-Reply-To: <1229001361-9301-1-git-send-email-git@fabian-franz.de>

Fabian Franz <git@fabian-franz.de> writes:

> So my workflow really is:
>
> git checkout master # done long before
> [...]
> git checkout staging
> # => in submodules/client/
> # Checked out submodules/client/ staging.
> # => in submodules/client/component1/
> # Checked out submodules/client/component/1 staging.
>
> So I would like to have this recursively...

Didn't we add foreach subcommand to the submodule command?

^ permalink raw reply

* Re: epic fsck SIGSEGV!
From: Linus Torvalds @ 2008-12-11 20:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, R. Tyler Ballance, Johannes Sixt, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0812110928060.3340@localhost.localdomain>



On Thu, 11 Dec 2008, Linus Torvalds wrote:
> 
> But obviously the true test for fsck is some complex corruption, and I 
> didn't test that. I can't imagine that it introduces any new problems 
> though - but the bugs you can't imagine are always the worst ones ;)

Btw, even if it doesn't introduce any bugs, it _does_ change the order 
that we traverse things in. It shouldn't matter, of course, but because it 
always picks the last entry from the object array (it really treats the 
array as a stack), it ends up traversing parents of commits (and the 
entries in trees) by looking at the last parent (or entry) first.

The whole two-phase thing also means that rather traverse the references 
as we find them, we'll end up traversing things later in one group. Again, 
access ordering will change.

Absolutely nothing should care about this from a correctness angle, of 
course, but I thought I'd point it out because I think it will change the 
order that we print out errors in.

So if somebody has some test-case, and you get different output 
before-and-after, it's not necessarily any indication of a problem, just 
an effect of doing object traversal in slightly different order.

		Linus

^ permalink raw reply

* Re: Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: Boyd Stephen Smith Jr. @ 2008-12-11 20:23 UTC (permalink / raw)
  To: git; +Cc: rdkrsr, Linus Torvalds
In-Reply-To: <d304880b0812110958u3da52e4fs7e5154ebe9a353a@mail.gmail.com>

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

On Thursday 2008 December 11 11:58:01 rdkrsr wrote:
>I'm not developing linux kernel, I just wanted to experiment with git.
>And then I didn't know if this is a normal behaviour of git. I'm using
>windows xp and msysgit for this. And the file system is NTFS.

You might want to choose a more MS Windows-friendly codebase to test with.  
For codebases without filenames that differ only in case, git should work 
fine on MS Windows.  For codebases with filenames that differ only in case, I 
don't know a VCS that will handle them well on MS Windows.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss03@volumehost.net                      ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.org/                      \_/     

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

^ permalink raw reply

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Robin Rosenberg @ 2008-12-11 20:23 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081211183954.GH32487@spearce.org>

torsdag 11 december 2008 19:39:54 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > > +		assertEquals(572, fRepositoryConfigTest.startOffset);
> > > +		assertEquals(1490, fRepositoryConfig.startOffset);
> >
> > 1487 here
> 
> Really?  1490 is the only value that the test vector passes with.
> What's the 3 bytes you think I'm off by?

Ah, --whitespace=fix did that.

-- robin

^ permalink raw reply

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Shawn O. Pearce @ 2008-12-11 20:27 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200812112123.49659.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> torsdag 11 december 2008 19:39:54 skrev Shawn O. Pearce:
> > Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > > > +		assertEquals(572, fRepositoryConfigTest.startOffset);
> > > > +		assertEquals(1490, fRepositoryConfig.startOffset);
> > >
> > > 1487 here
> > 
> > Really?  1490 is the only value that the test vector passes with.
> > What's the 3 bytes you think I'm off by?
> 
> Ah, --whitespace=fix did that.

Ok.  I know you like to apply with --whitespace=fix, but I would
prefer to leave these *.patch test input files[*1*] exactly as they
were created by git format-patch or git diff, so I'm sure we are
parsing the same thing git would have produced and sent to us.


*1*: I have more patches coming which add 2 more test inputs
     to the same PatchTest suite.

-- 
Shawn.

^ permalink raw reply

* git-doc CSS dependent, breaks down in text browsers
From: jidanni @ 2008-12-11 20:29 UTC (permalink / raw)
  To: git
In-Reply-To: <87ej0qf3gx.fsf@jidanni.org>

E.g., pages look like

SYNOPSIS

git-config [<file-option>] [type] [-z|--null] name [value [value_regex]] git-config [<file-option>] [type] --add name
value git-config [<file-option>] [type] --replace-all name [value [value_regex]] git-config [<file-option>] [type] [-z|
--null] --get name [value_regex] git-config [<file-option>] [type] [-z|--null] --get-all name [value_regex] git-config...

Please see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507475 ,
which was supposed to be forwarded to git@vger.kernel.org but wasn't,
apparently.

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2008, #06; Wed, 26)
From: Daniel Barkalow @ 2008-12-11 20:30 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, git
In-Reply-To: <fcaeb9bf0812110504u1acfb612he3edae1df3774045@mail.gmail.com>

On Thu, 11 Dec 2008, Nguyen Thai Ngoc Duy wrote:

> On 12/9/08, Daniel Barkalow <barkalow@iabervon.org> wrote:
> >  >  - for "git grep", we ignore path with CE_NO_CHECKOUT (while using
> >  > cache version for CE_VALID)
> >
> >
> > Is this sufficient? I'd expect "git grep" to ignore paths that are outside
> >  the checked-out region, even when searching an arbitrary tree, and even
> >  when those files aren't in the index at all (i.e., the current commit
> >  doesn't have them). That is, I'd expect core.defaultsparse or the
> >  equivalent to limit the paths, normally giving this effect.
> 
> That's the point. CE_VALID does not define checkout area while
> CE_NO_CHECKOUT does.  If an entry is CE_VALID, it is still in checkout
> area. But if it is CE_NO_CHECKOUT, "git grep" should ignore that path.
> core.defaultsparse has nothing to do here.

My point is that the index cannot tell git grep whether it should search a 
path if the path isn't in the index. If I do a narrow checkout of only 
Documentation/, and I do "git grep foo", I won't see files that aren't in 
Documentation/; if I do "git grep foo origin/next", I think I shouldn't 
see files that aren't in Documentation/, and "new-program.c" isn't in my 
index at all, marked as CE_NO_CHECKOUT or otherwise, so git grep can't 
find out from the index whether that file is outside my area of interest. 
It needs to be able to determine that "only Documentation/ is in the 
checkout area" ignoring the details of the list of files in the working 
directory currently in or out of the area.

> >  > >  The question, then, is what happens when the index and core.defaultsparse
> >  > >  disagree, either because the porcelain supports causing it or because the
> >  > >  user has simply editting the config file or used plumbing to modify the
> >  > >  index. That is, (1) we have index entries that say that the worktree is
> >  > >  ignored, and the rules don't say they're outside the sparse checkout; do
> >  > >  we care whether we expect the worktree to be empty or match the index?
> >  > >  And, (2) we have index entries that say we do care about them, but the
> >  > >  rules say they're outside the sparse checkout; what happens with these?
> >  >
> >  > The rule is CE_NO_CHECKOUT is king. core.defaultsparse only helps
> >  > setting CE_NO_CHECKOUT on new entries when they enter the index.
> >
> >
> > This seems like a really bad idea to me. If you ask for a file that's
> >  outside your default area to be checked out, and then you switch branches
> >  and switch back, the file may or may not disappear (depending on whether
> >  the branch you switched to temporarily had it or not). Likewise, if you
> >  remove files, and then switch branches and back, the files may or may not
> >  reappear.
> 
> Well, if you set core.defaultsparse properly, those files should
> appear/disappear as you wish (and as of now if you define your
> checkout area with "git checkout --{include-,exclude-,}sparse" then
> core.defaultsparse should be updated accordingly). I don't say
> core.defaultsparse is perfect.

Right, so in order to get reasonable behavior, the user must use 
--{include,exclude}-sparse. I think that this should be the *default* 
behavior, and probably the *only porcelain-supported* behavior, because 
otherwise it's confusing.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* user-manual.html invalid HTML
From: jidanni @ 2008-12-11 20:32 UTC (permalink / raw)
  To: git
In-Reply-To: <87ej0qf3gx.fsf@jidanni.org>

Please see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507476
Which it turns out didn't get forwarded to git@vger.kernel.org after all. 

^ permalink raw reply

* Re: Fwd: after first git clone of linux kernel repository there are changed files in working dir
From: Giuseppe Bilotta @ 2008-12-11 20:35 UTC (permalink / raw)
  To: git
In-Reply-To: <d304880b0812110958u3da52e4fs7e5154ebe9a353a@mail.gmail.com>

On Thursday 11 December 2008 18:58, rdkrsr wrote:

> Thank you, Linus and Brett, for your answers.
> 
> I'm not developing linux kernel, I just wanted to experiment with git.
> And then I didn't know if this is a normal behaviour of git. I'm using
> windows xp and msysgit for this. And the file system is NTFS. I'm
> using dual boot to sporadicly use linux and tried also linux in
> virtual box. But both isn't really good. Maybe one day I dare to use
> linux as my primary OS.
> 
> Red
> 
> 2008/12/11 Linus Torvalds <torvalds@linux-foundation.org>:
>>
>>
>> On Thu, 11 Dec 2008, rdkrsr wrote:
>>>
>>> I'm sorry that I didn't answer to git mailing list address. So here
>>> comes the email again.
>>
>> You have a broken filesystem.

Actually, the funny (in a grotesque kind of way) thing about NTFS
is that it's a case-*sensitive* filesystem (in the sense that i
can legally hold files with names that differ only for the case),
but the Windows subsystems use it in case-preserving (but
insensitive) mode. It is possible to use NTFS case insensitively
under Windows, but it requires something such as the Interix
subsystem, and in Windows XP (and possibly later versions) it also
needs toggling a security policy that defaults to enforcing case
insensitivity for all subsystems.

Maybe the Windows ports to git (at least the cygwin one, maybe?)
may be able to exploit this.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* [PATCH] autodetect number of CPUs by default when using threads
From: Nicolas Pitre @ 2008-12-11 20:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

... and display the actual number of threads used when locally 
repacking.  A remote server still won't tell you how many threads it 
uses during a fetch though.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

I've spent quite a while wondering why repacking in one repo was faster 
than repacking in a clone of that repo on the same machine.  So let's 
display how many threads are actually used.

We have comprehensive test in Makefile to determine if threads are 
available, just to not use them by default.  I think that code has 
proven itself for long enough now not to let people benefit from it.

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index cedef52..619e597 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -78,7 +78,7 @@ static int progress = 1;
 static int window = 10;
 static uint32_t pack_size_limit, pack_size_limit_cfg;
 static int depth = 50;
-static int delta_search_threads = 1;
+static int delta_search_threads;
 static int pack_to_stdout;
 static int num_preferred_base;
 static struct progress *progress_state;
@@ -1612,6 +1612,9 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 		find_deltas(list, &list_size, window, depth, processed);
 		return;
 	}
+	if (progress > pack_to_stdout)
+		fprintf(stderr, "Delta compression using %d threads.\n",
+				delta_search_threads);
 
 	/* Partition the work amongst work threads. */
 	for (i = 0; i < delta_search_threads; i++) {

^ permalink raw reply related

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Robin Rosenberg @ 2008-12-11 20:39 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081211183954.GH32487@spearce.org>

torsdag 11 december 2008 19:39:54 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> > > +		assertEquals(572, fRepositoryConfigTest.startOffset);
> > > +		assertEquals(1490, fRepositoryConfig.startOffset);
> >
> > 1487 here
> 
> Really?  1490 is the only value that the test vector passes with.
> What's the 3 bytes you think I'm off by?

...Quick, quick, find something else to pick on..  :->

Yes. Very little of the code in TemporaryBuffer is covered by the unit tests
and number of conditionals in there are rather large. I tried messing with
the constants in there to improve that and then PatchTest started to fail.

Here are the changes I tried with. I think it should still work with thes
changes. Rather than changing the other tests, we might want to create
a special test for only the buffer class.

-- robin

diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
index 27f6444..556ab71 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
@@ -60,7 +60,7 @@
  * after this stream has been properly closed by {@link #close()}.
  */
 public class TemporaryBuffer extends OutputStream {
-       private static final int DEFAULT_IN_CORE_LIMIT = 1024 * 1024;
+       private static final int DEFAULT_IN_CORE_LIMIT = 1024;

        /** Chain of data, if we are still completely in-core; otherwise null. */
        private ArrayList<Block> blocks;
@@ -315,7 +315,7 @@ public void destroy() {
        }

        private static class Block {
-               static final int SZ = 8 * 1024;
+               static final int SZ = 512;

                final byte[] buffer = new byte[SZ];

^ permalink raw reply related

* Re: [JGIT PATCH 4/5] Define Patch to parse a sequence of patch FileHeaders
From: Shawn O. Pearce @ 2008-12-11 20:41 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200812112139.29875.robin.rosenberg.lists@dewire.com>

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> ...Quick, quick, find something else to pick on..  :->

Heh, you'll get more patches again from me today, so there'll still
be more to pick on. :)
 
> Yes. Very little of the code in TemporaryBuffer is covered by the unit tests
> and number of conditionals in there are rather large. I tried messing with
> the constants in there to improve that and then PatchTest started to fail.
> 
> Here are the changes I tried with. I think it should still work with thes
> changes. Rather than changing the other tests, we might want to create
> a special test for only the buffer class.

Ok.  I was thinking the same thing actually, that I should spend a
bit of time today and try to get coverage on TemporaryBuffer.  I'll
write a unit test for it.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v3] submodule: Allow tracking of the newest revision of a branch in a submodule
From: Junio C Hamano @ 2008-12-11 20:42 UTC (permalink / raw)
  To: Fabian Franz; +Cc: git, hjemli, j.sixt
In-Reply-To: <1229009982-2701-1-git-send-email-git@fabian-franz.de>

Fabian Franz <git@fabian-franz.de> writes:

> Submodules currently only allow tracking a specific revision
> and each update in a submodule leads to a new commit in the
> master repository. However some users may want to always track
> the newest revision of a specific (named) tag or branch or HEAD.
> For example the user might want to track a staging branch in all
> submodules.

I initially liked the direction this is going, but I think the above
rationale and the change to use 0{40} have impedance mismatch.  Your
change is not a good way to go about "some users may want".  I'll discuss
more on this below.

> To allow this the "--track|-t <branch>" parameter was added to
> git-submodule.sh, which is added to .gitmodules config file as
> well as "track" parameter. This creates a new local branch on
> checkout, which is tracking the remote branch in case the local
> branch does not yet exist.
>
> Technically the gitlink code was changed to always compare
> successful (so no changes) in case the sha1 is null. In that
> case no new commit is created when there are changes in the
> submodule.

"Technically" here sounds wrong.  I'd suggest dropping it, e.g. "Update
ce_compare_gitlink() so that it always reports a match if the commit
recorded in the index is a null SHA-1."

Because I also do not see a direct connection between "no new commit is
created" and "there are changes in the submodule", I think the last
sentence in the above paragraph of yours is misleading.  The user _can_
create A new commit in the superproject that records a different gitlink
from its parent commit, if (and only if) the user wishes to bind the
updated subproject's branch state to the new state of the superproject,
and that is done by adding the subproject status to the staging area with
"git add" (or use "git commit -a").

It seems to me that what you are really after is to let you change the
state of the subproject checkout in whatever way and have "git commit -a"
in the superproject ignore that change.

I wonder if you can just set "assume unchanged" bit for the subproject
gitlink in the index to achieve the same goal.

Or is there more to it?

> The submodule code is adding the file with 0000* on
> "add".
>
> Signed-off-by: Fabian Franz <git@fabian-franz.de>
> ---
>
> I like this and because of that the --branch is optional. I also like
> that so much, that we have decided against Google Repo.
>
> However I have both cases: Stable development, where I need one special
> version and "wild" development, where I always want the newest published
> one.

I do not think supporting both styles of development is a bad idea.

However, use of 0{40} in the index and the resulting commit object in the
superproject means that this is a project-wide decision, not your personal
preference.  It is not implausible that you would want to do a wild
expeeriment in your own clone of a project that uses the "Stable
development" approach (hence the upstream never would want to have 0{40}
gitlink in its commits).

For example, suppose the project uses "Stable development" approach, and
records the v1.0.0 of submodule at "sub/" in the superproject.  You are a
contributor to that project, and would want to help them futureproof the
superproject code to be forward compatible with the upcoming v1.2.0
release of the subproject.  What would you do?

 * have a clone of superproject, with v1.0.0 submodule bound at "sub/";

 * go to "sub/", fetch and checkout v1.2.0-rc2;

 * go up, build using the updated submodule, see many failures in
   supermodule build;

 * fix them up in a way that can work with both v1.0.0 and v1.2.0 of the
   submodule, while making commits in logical steps, in the supermodule.

And you do not want to record the fact that you used v1.2.0-rc2 of the
submodule at "sub/" in the commits you make in the supermodule, as you
would want to label these commits as "futureproof for upcoming submodule
v1.2.0".

But you cannot use your 0{40} trick, as sending that to the upstream of
the superproject would break their "Stable development" policy.

I wonder if you can just set "assume unchanged" bit for the subproject
gitlink in the index to achieve the same goal.  That would be a local
operation, the gitlink would still point at v1.0.0 version of submodule,
and "git commit -a" in the superproject won't make commits that flips
everybody else's copy to use v1.2.0-rc2 of submodule.

>>I've reviewed the patch just from a shell code writer's point of view.
>
> Okay, I added your suggestions.

In the commentary section in your v2 patch, you said "However I see
problems on remove".  Has that issue been addressed?

> @@ -118,6 +118,10 @@ update::
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
>  submodule with the --init option.
> ++
> +If you used --track or set the "track" option in .gitmodules this will
> +automatically pull the newest updates from remote instead of tracking a
> +specific revision.

"automatically pull" in the sense that it always goes to the remote, fetch
and merge?  That sounds horribly broken.  You can never work disconnected?

> @@ -159,6 +163,10 @@ OPTIONS
>  --branch::
>  	Branch of repository to add as submodule.
>  
> +-t::
> +--track::
> +	Branch/Tag/HEAD of repository to track in a submodule.
> +

How does the branch parameter to the --track option interact with the
branch parameter to the --branch option?  Does an end user typically set
them to the same branch?  Or would these parameters almost always point at
different branchesof the remote repository?  What are the reasons for the
end user to choose one parameter value for the --branch option and a
different parameter value for the --track option?

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2f47e06..16df528 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> ...
> @@ -197,12 +203,14 @@ cmd_add()
>  		(unset GIT_DIR; cd "$path" && git checkout -f -q ${branch:+-b "$branch" "origin/$branch"}) ||
>  		die "Unable to checkout submodule '$path'"
>  	fi
> +	test -n "$track" && printf '160000 0000000000000000000000000000000000000000\t%s\n' "$path" | git update-index --index-info
>  

You have many overlong lines due to the 0{40} constant string in your
patch.  Have a

	null_sha1=0000000000000000000000000000000000000000

at the beginning of the script, and rewrite the above like this:

	test -n "$track" &&
                printf '160000 %s\t%s\n' "$null_sha1" "$path" |
                git update-index --index-info

or even like this:

	if test -n "$track"
        then
                printf '160000 %s\t%s\n' "$null_sha1" "$path" |
                git update-index --index-info
	fi

Use of $null_sha1 throughout the script will make things easier to read
and at the same time make it less error prone as well for "git submodule"
developers.

> @@ -345,11 +357,29 @@ cmd_update()
>  			then
>  				force="-f"
>  			fi
> +			pull=
> +			if [ "$sha1" = "0000000000000000000000000000000000000000" ]
> +			then
> +				track=$(git config submodule."$name".track)
> +				: ${track:="master"}

In the v2 patch this used to point at "HEAD".  What made you change your
mind?

> +				# if the local branch does not yet exist, create it
> +				( unset GIT_DIR; cd "$path"; git-show-ref --heads --tags -q "$track" || git branch --track "$track" "origin/$track" )

No error checking?

	(
        	unset GIT_DIR;
                cd "$path" &&
                git show-ref --heads --tags -q "$track" ||
                git branch --track "$track" "origin/$track"
	) || barf

The ';' after unset is intentional; some shells reports failure when you
unset an unset variable.

> +				sha1="$track"
> +				pull=1

I tend to prefer booleans in shell scripts spelled like boolean, e.g.

	pull=yes

> +			fi
> +
>  			(unset GIT_DIR; cd "$path" && git-fetch &&
>  				git-checkout $force -q "$sha1") ||
>  			die "Unable to checkout '$sha1' in submodule path '$path'"
>  
>  			say "Submodule path '$path': checked out '$sha1'"
> +
> +			if [ "$pull" = "1" ]
> +			then
> +				# Now pull new updates from origin
> +				( unset GIT_DIR; cd "$path"; git-pull ) || die "Unable to pull in submodule path '$path'"

No error checking?


> +			fi
> +
>  		fi
>  	done
>  }
> @@ -596,7 +626,8 @@ cmd_status()
>  		set_name_rev "$path" "$sha1"
>  		if git diff-files --quiet -- "$path"
>  		then
> -			say " $sha1 $path$revname"
> +			track=$(git config submodule."$name".track)
> +			say " $sha1 $path$revname${track:+ (tracking "$track")}"
>  		else
>  			if test -z "$cached"
>  			then
> diff --git a/read-cache.c b/read-cache.c
> index 8579663..0c14b68 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -137,6 +137,11 @@ static int ce_compare_gitlink(struct cache_entry *ce)
>  	 */
>  	if (resolve_gitlink_ref(ce->name, "HEAD", sha1) < 0)
>  		return 0;
> +
> +	// To be able to track newest revision
> +	if (is_null_sha1(ce->sha1))
> +		return 0;
> +

I think the comment is wrong, as it is not about newness at all.

	/* ignore changes in the submodule path */

would be more appropriate.

^ permalink raw reply

* Re: Clarifying "invalid tag signature file" error message from git filter-branch (and others)
From: Brandon Casey @ 2008-12-11 21:06 UTC (permalink / raw)
  To: James Youngman; +Cc: git
In-Reply-To: <c5df85930812110214k2e12d926m60856fb630d45e80@mail.gmail.com>

James Youngman wrote:
> What do the errors "error: char88: malformed tagger field" and "fatal:
> invalid tag signature file" and "Could not create new tag object for
> FINDUTILS-4_1-10" signify in the session below?

It means the tagger field in the tag does not follow the correct form.
Specifically the testing in git-mktag (called by filter-branch) does:

         * Check for correct form for name and email
         * i.e. " <" followed by "> " on _this_ line
         * No angle brackets within the name or email address fields.
         * No spaces within the email address field.

What does 'git cat-file tag FINDUTILS-4_1-10' show you?

> Are any of those errors correctable (I can re-run the tree rewrite
> script as many times as needed, I'm just using it on a test repository
> for now).

If there are only a few, then you can manually retag with a corrected
tagger field, and then run your script.

Of course, depending on the output of the cat-file call above, the
testing in git-mktag may need to be relaxed.

-brandon

^ permalink raw reply

* [RFC] cgit in git?
From: Lars Hjemli @ 2008-12-11 21:48 UTC (permalink / raw)
  To: Junio C Hamano, Seth Vidal, Git Mailing List

Background: I've been asked by the fedora project how to package cgit.
The problem is basically that cgit is designed to be statically linked
with a specific git release (i.e. libgit.a and xdiff/lib.a).

When manually building cgit from a tarball this isn't a problem:
'make get-git' will download the required git sources from kernel.org.
But the buildsystem/policy used by the fedora project does not allow
network access during package builds, and since it is quite unlikely
that the git package always will match the exact release needed by the
cgit package, I only see four options:
1) the fedora project makes a 'git-for-cgit' package containing the
needed release of the git sources
2) the cgit release tarballs includes the needed git sources
3) the cgit sources are subtree-merged into git
4) cgit is modified to link against libgit2

Option 1 seems unlikely to happen since such a 'git-for-cgit' package
would basically require the fedora project to support two git
packages.

Option 2 is doable but still requires the fedora project to support
two git packages (but now the 'git-for-cgit' package is hidden inside
the cgit source tree). The good thing about this option is that it
only requires some minor modifications to the cgit releases.

Option 3 would solve the problem for the fedora project but is not for
me to decide - it might become an extra maintenance burden on the git
maintainer and community.

Option 4 is the correct solution but not a very practical one; it's
currently hard to predict when libgit2 will be ready for general
(c)git use.

Personally I'd love for option 3 to happen, hence this rfc.

-- 
larsh

^ 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