* [StGIT PATCH 5/5] Speed up the appliedness test
From: Karl Hasselström @ 2007-08-07 2:48 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <20070807024508.11373.62875.stgit@yoghurt>
The appliedness test was too slow if at least one patch, applied or
unapplied, was too far away from HEAD, since we had to visit the whole
intervening part of the commit DAG.
This patch fixes that problem by maintaining a cache of uninteresting
commits that are known to not reach any patches in the commit DAG.
(Specifically, this is at all times the set of commits that are
parents to patch commits and do not have a patch commit as their
ancestor.) By exlcuding these commits when walking the graph, we only
have to visit the interesting places.
As a nice side effect, the cache of uninteresting commits makes it
possible to use just one git-rev-list call instead of two, since we
can list the applied patches without first computing the unapplied
patches; the unapplied patches are then simply all patches except
those that are applied.
Signed-off-by: Karl Hasselström <kha@treskal.com>
---
stgit/stack.py | 272 ++++++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 221 insertions(+), 51 deletions(-)
diff --git a/stgit/stack.py b/stgit/stack.py
index 4186ba9..5a51329 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -155,7 +155,7 @@ class Patch(StgitObject):
os.mkdir(self._dir())
self.create_empty_field('bottom')
self.create_empty_field('top')
-
+
def delete(self):
for f in os.listdir(self._dir()):
os.remove(os.path.join(self._dir(), f))
@@ -351,7 +351,11 @@ class PatchorderCache:
saved in its patchorder file."""
def __init__(self, series):
self.__series = series
+ self.__file = os.path.join(self.__series._dir(), 'patchorder')
self.__invalidate()
+ def delete_file(self):
+ if os.path.isfile(self.__file):
+ os.remove(self.__file)
def __invalidate(self):
self.__patchnames = None
self.__position = None
@@ -361,9 +365,8 @@ class PatchorderCache:
self.__patchnames = []
self.__position = {}
- pof = os.path.join(self.__series._dir(), 'patchorder')
- if os.path.isfile(pof):
- for line in file(pof):
+ if os.path.isfile(self.__file):
+ for line in file(self.__file):
name = line.strip()
assert not name in self.__position
self.__position[name] = len(self.__patchnames)
@@ -386,60 +389,200 @@ class PatchorderCache:
pos2 = self.__position.get(name2, largepos)
return cmp((pos1, name1), (pos2, name2))
+class UninterestingCache:
+ """Keeps track of a set of commits that do not reach any patches.
+ These are used to speed up the detection of unapplied patches.
+
+ Specifically, this is at all times the set of commits c that
+ fulfill the following two criteria:
+
+ * c does not reach any patch
+
+ * c is the parent of a patch
+
+ """
+ def __init__(self, series):
+ self.__series = series
+ self.__uninteresting = None
+ self.__filename = os.path.join(self.__series._dir(), 'uninteresting')
+ def __invalidate(self):
+ self.__uninteresting = None
+ self.delete_file()
+ def delete_file(self):
+ if os.path.isfile(self.__filename):
+ os.remove(self.__filename)
+ def __other_patches(self, patchname):
+ """All patches except the named one."""
+ ref2hash = read_refs(self.__series.get_name())
+ return [self.__series.get_patch(ref)
+ for ref in ref2hash.iterkeys()
+ if ref and ref != patchname]
+ def __write_file(self):
+ """Write the uninteresting commits to file."""
+ try:
+ f = file(self.__filename, 'w')
+ for u in self.__uninteresting:
+ f.write('%s\n' % u)
+ f.close()
+ except IOError:
+ pass # this isn't fatal -- the directory is probably missing
+ def __read_file(self):
+ """Read the uninteresting commits from file. Return true on
+ success, false on failure."""
+ if not os.path.isfile(self.__filename):
+ return False
+ self.__uninteresting = Set()
+ for line in file(self.__filename):
+ self.__uninteresting.add(line.strip())
+ return True
+ def __cache_file(self):
+ """Try to cache the uninteresting commits using only the cache
+ file. Return true on success, false on failure."""
+ if self.__uninteresting != None:
+ return True # already cached
+ return self.__read_file()
+ def __cache(self):
+ """Cache the uninteresting commits, recomputing them if
+ necessary."""
+ if self.__cache_file():
+ return
+ self.__compute_uninteresting()
+ self.__write_file()
+ def __compute_uninteresting(self):
+ """Compute a reasonable set of uninteresting commits from
+ scratch. This is expensive."""
+ out.start('Finding uninteresting commits')
+ ref2hash = read_refs(self.__series.get_name())
+ patches = Set([sha1 for ref, sha1 in ref2hash.iteritems() if ref])
+ interesting, uninteresting = Set(), Set()
+
+ # Iterate over all commits. We are guaranteed to see each
+ # commit before any of its children.
+ for line in git._output_lines(
+ 'git-rev-list --topo-order --reverse --parents --all'):
+ commits = line.split()
+ commit, parents = commits[0], Set(commits[1:])
+
+ # Patches are interesting.
+ if commit in patches:
+ interesting.add(commit)
+
+ # The parents of a patch are uninteresting unless they
+ # are interesting.
+ for p in parents:
+ if not p in interesting:
+ uninteresting.add(p)
+ continue
+
+ # Commits with interesting parents are interesting.
+ if interesting.intersection(parents):
+ interesting.add(commit)
+ self.__uninteresting = uninteresting
+ out.done()
+ def create_patch(self, name, top, bottom):
+ """The given patch has been created. Update the uninterested
+ state to maintain the invariant."""
+ if not self.__cache_file():
+ return # not cached
+
+ # New patch inserted just below an existing bottommost patch:
+ # need to move the uninteresting commit down one step.
+ if top in self.__uninteresting:
+ self.__uninteresting.remove(top)
+ self.__uninteresting.add(bottom)
+ self.__write_file()
+ return
+
+ # New patch inserted next to an existing non-bottommost patch:
+ # don't need to do anything.
+ existing_patches = self.__other_patches(name)
+ tops = Set([p.get_top() for p in existing_patches])
+ bottoms = Set([p.get_bottom() for p in existing_patches])
+ if bottom in bottoms or bottom in tops or top in bottoms:
+ return
+
+ # The new patch is not adjacent to an existing patch. We'd
+ # need to first get rid of any uninteresting commit that
+ # reaches this patch, and then mark the patch's bottom
+ # uninteresting if it doesn't reach any other patch. This is a
+ # lot of work, so we chicken out and blow the whole cache
+ # instead.
+ self.__invalidate()
+ def delete_patch(self, name, top, bottom):
+ """The given patch has been deleted. Update the uninterested
+ state to maintain the invariant."""
+ if not self.__cache_file():
+ return # not cached
+
+ # If this patch reaches another patch, there's nothing to do.
+ if not bottom in self.__uninteresting:
+ return
+
+ # If another patch has the same bottom, it's still
+ # uninteresting and there's nothing more to do.
+ other_patches = self.__other_patches(name)
+ for p in other_patches:
+ if p.get_bottom() == bottom:
+ return
+
+ # If there are other patches on top of this one, their bottoms
+ # (this patch's top) become uninteresting in place of this
+ # patch's bottom.
+ for p in other_patches:
+ if p.get_bottom() == top:
+ self.__uninteresting.remove(bottom)
+ self.__uninteresting.add(top)
+ self.__write_file()
+ return
+
+ # The bottom of this patch is no longer uninteresting. But
+ # there might be other patches that reach it, whose bottoms
+ # would need to be marked uninteresting. That would require an
+ # expensive reachability analysis.
+ self.__invalidate()
+ def get(self):
+ self.__cache()
+ return self.__uninteresting
+
def read_refs(branch):
"""Return a mapping from patches and branch head to hashes for a
given branch. The patches are listed by name; the branch head is
None."""
refs = {}
patchpat = re.compile(r'^refs/patches/%s/([^\.]+)$' % branch)
+ head = 'refs/heads/%s' % branch
for line in git._output_lines('git-show-ref'):
sha1, ref = line.split()
m = re.match(patchpat, ref)
if m:
refs[m.group(1)] = sha1
- elif ref == 'refs/heads/%s' % branch:
+ elif ref == head:
refs[None] = sha1
+ if not None in refs:
+ raise StackException, 'Could not find %s' % head
return refs
-def unapplied_patches(ref2hash):
+def get_patches(ref2hash, uninteresting):
"""Given a map of patch names (and the branch head, keyed by None)
- to hashes, return the set of unapplied patches."""
- hash2refs = {}
- for r, h in ref2hash.iteritems():
- hash2refs.setdefault(h, Set()).add(r)
-
+ to hashes, return the list of applied patches and the set of
+ unapplied patches. The second parameter is a set of commit objects
+ that do not reach any patch."""
+ applied = []
unapplied = Set()
- for line in git._output_lines(
- 'git-rev-list --stdin',
- ('%s%s\n' % (['', '^'][ref == None], sha1)
- for ref, sha1 in ref2hash.iteritems())):
- for ref in hash2refs.get(line.strip(), []):
- unapplied.add(ref)
- return unapplied
-
-def sort_applied_patches(ref2hash):
- """Given a map of patch names (and the branch head, keyed by None)
- to hashes, return a list with the applied patches in stack order.
- All patches in the map must be applied."""
- hash2refs = {}
+ hash2patches = {}
for r, h in ref2hash.iteritems():
- if r != None:
- hash2refs.setdefault(h, Set()).add(r)
+ if r:
+ hash2patches.setdefault(h, Set()).add(r)
+ unapplied.add(r)
- missing = Set(ref for ref in ref2hash.iterkeys() if ref != None)
- if not missing:
- return []
- applied = []
- grl = popen2.Popen3('git-rev-list %s' % ref2hash[None], True)
- for line in grl.fromchild:
- for ref in hash2refs.get(line.strip(), []):
+ for line in git._output_lines(
+ 'git-rev-list --topo-order --stdin', ['%s\n' % ref2hash[None]]
+ + ['^%s\n' % u for u in uninteresting]):
+ for ref in hash2patches.get(line.strip(), []):
applied.append(ref)
- missing.remove(ref)
- if not missing:
- applied.reverse()
- return applied
-
- raise StackException, 'Could not find patches: %s' % ', '.join(missing)
+ unapplied.remove(ref)
+ applied.reverse()
+ return applied, unapplied
class AppliedCache:
"""An object that keeps track of the appliedness and order of the
@@ -447,7 +590,11 @@ class AppliedCache:
def __init__(self, series):
self.__series = series
self.__order = PatchorderCache(series)
+ self.__uninteresting = UninterestingCache(series)
self.__invalidate()
+ def delete_files(self):
+ for sub in [self.__uninteresting, self.__order]:
+ sub.delete_file()
def get_applied(self):
self.__cache()
return self.__applied
@@ -466,6 +613,17 @@ class AppliedCache:
self.__write_patchorder()
return
raise StackException, 'Unknown patch "%s"' % oldname
+ def new(self, name, top, bottom):
+ """Create new patch."""
+ self.__uninteresting.create_patch(name, top, bottom)
+ def delete(self, name, top, bottom):
+ """Delete a patch."""
+ self.__uninteresting.delete_patch(name, top, bottom)
+ def change(self, name, old_top, old_bottom, new_top, new_bottom):
+ """Change a patch."""
+ if (new_top, new_bottom) != (old_top, old_bottom):
+ self.new(name, new_top, new_bottom)
+ self.delete(name, old_top, old_bottom)
def __write_patchorder(self):
self.__order.set_patchorder(self.get_applied() + self.get_unapplied())
def set_patchorder(self, new_order):
@@ -484,11 +642,8 @@ class AppliedCache:
def __cache(self):
if self.__cached():
return
- patches = read_refs(self.__series.get_name())
- unapplied = unapplied_patches(patches)
- for patch in unapplied:
- del patches[patch]
- self.__applied = sort_applied_patches(patches)
+ self.__applied, unapplied = get_patches(
+ read_refs(self.__series.get_name()), self.__uninteresting.get())
self.__unapplied = list(unapplied)
self.__unapplied.sort(self.__order.cmp)
@@ -849,6 +1004,7 @@ class Series(PatchSet):
os.remove(self.__hidden_file)
if os.path.exists(self._dir()+'/orig-base'):
os.remove(self._dir()+'/orig-base')
+ self.__applied_cache.delete_files()
if not os.listdir(self.__patch_dir):
os.rmdir(self.__patch_dir)
@@ -953,16 +1109,20 @@ class Series(PatchSet):
patch = self.get_patch(name)
old_bottom = patch.get_old_bottom()
old_top = patch.get_old_top()
+ curr_bottom = patch.get_bottom()
+ curr_top = patch.get_top()
# the bottom of the patch is not changed by refresh. If the
# old_bottom is different, there wasn't any previous 'refresh'
# command (probably only a 'push')
- if old_bottom != patch.get_bottom() or old_top == patch.get_top():
+ if old_bottom != curr_bottom or old_top == curr_top:
raise StackException, 'No undo information available'
git.reset(tree_id = old_top, check_out = False)
if patch.restore_old_boundaries():
self.log_patch(patch, 'undo')
+ self.__applied_cache.change(name, curr_top, curr_bottom,
+ old_top, old_bottom)
def new_patch(self, name, message = None, can_edit = True,
unapplied = False, show_patch = False,
@@ -995,10 +1155,9 @@ class Series(PatchSet):
patch = self.get_patch(name)
patch.create()
- if not bottom:
- bottom = head
- if not top:
- top = head
+ bottom = bottom or head
+ top = top or head
+ self.__applied_cache.new(name, top, bottom)
patch.set_bottom(bottom)
patch.set_top(top)
@@ -1042,15 +1201,16 @@ class Series(PatchSet):
def delete_patch_data(self, name):
"""Deletes the stgit data for a patch."""
patch = Patch(name, self.__patch_dir, self.__refs_dir)
+ top, bottom = patch.get_top(), patch.get_bottom()
# save the commit id to a trash file
- write_string(os.path.join(self.__trash_dir, name), patch.get_top())
+ write_string(os.path.join(self.__trash_dir, name), top)
patch.delete()
if self.patch_hidden(name):
self.unhide_patch(name)
- return patch
+ self.__applied_cache.delete(name, top, bottom)
def delete_patch(self, name):
"""Deletes a patch
@@ -1109,6 +1269,7 @@ class Series(PatchSet):
top_tree = git.get_commit(top).get_tree()
+ old_top = top
top = git.commit(message = descr, parents = [head],
cache_update = False,
tree_id = top_tree,
@@ -1122,6 +1283,9 @@ class Series(PatchSet):
patch.set_bottom(head, backup = True)
patch.set_top(top, backup = True)
+ self.__applied_cache.change(
+ name, old_top = old_top, old_bottom = bottom,
+ new_top = top, new_bottom = head)
self.log_patch(patch, 'push(f)')
else:
top = head
@@ -1179,6 +1343,7 @@ class Series(PatchSet):
# need an empty commit
patch.set_bottom(head, backup = True)
patch.set_top(head, backup = True)
+ self.__applied_cache.change(name, top, bottom, head, head)
modified = True
elif head == bottom:
# reset the backup information. No need for logging
@@ -1191,6 +1356,7 @@ class Series(PatchSet):
# The current patch is empty after merge.
patch.set_bottom(head, backup = True)
patch.set_top(head, backup = True)
+ self.__applied_cache.change(name, top, bottom, head, head)
# Try the fast applying first. If this fails, fall back to the
# three-way merge
@@ -1236,6 +1402,8 @@ class Series(PatchSet):
patch = self.get_patch(name)
old_bottom = patch.get_old_bottom()
old_top = patch.get_old_top()
+ curr_bottom = patch.get_bottom()
+ curr_top = patch.get_top()
# the top of the patch is changed by a push operation only
# together with the bottom (otherwise the top was probably
@@ -1247,6 +1415,8 @@ class Series(PatchSet):
git.reset()
self.pop_patch(name)
ret = patch.restore_old_boundaries()
+ self.__applied_cache.change(name, curr_top, curr_bottom,
+ old_top, old_bottom)
if ret:
self.log_patch(patch, 'undo')
^ permalink raw reply related
* Re: Problem with bisect
From: Larry Finger @ 2007-08-07 2:53 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <200708070350.50419.chriscool@tuxfamily.org>
Christian Couder wrote:
>
> You use "git bisect good v2.6.22", but this is not true because the
> tag "v2.6.22" is on the mainstream kernel branch and the driver is not
> there.
>
> If the v2.6.22 kernel that used to work came directly from John Linville's
> wireless-dev git tree, not from a patch, then you should find the exact
> commit in John Linville's tree that worked and say "git bisect good <this
> commit>".
>
> But if the driver that worked with a mainstream v2.6.22 kernel had been
> patched, and now doesn't work when the same patch is applied to mainstream
> v2.6.23-rc1 kernel, then you can perhaps use:
>
> git bisect start
> git bisect bad v2.6.23-rc1
> git bisect good v2.6.22
>
> and then:
>
> 1) patch the kernel with the driver patch,
> 2) test the patched kernel,
> 3) remove the patch,
> 4) say "git bisect good" or "git bisect bad"
> 5) go to step 1) until the commit that broke the driver is found
Has the ability to use a commit hash to indicate a start point been in git for a long time? I think
I remember trying it before when a version tag had not been downloaded and having a failure.
Larry
^ permalink raw reply
* Re: [PATCH] unpack-trees.c: assume submodules are clean during check-out
From: Eran Tromer @ 2007-08-07 3:24 UTC (permalink / raw)
To: skimo; +Cc: Junio C Hamano, git
In-Reply-To: <20070806190344.GF999MdfPADPa@greensroom.kotnet.org>
On 2007-08-06 15:03, Sven Verdoolaege wrote:
> I don't see the difference. If you forgot you changed something
> (be it a submodule or a file) you will commit something you
> didn't plan to commit.
...
> bash-3.00$ git checkout master && echo "test" > b && git commit -a -m 'change b'
> M c
> Switched to branch "master"
> Created commit 657c5b1: change b
> 2 files changed, 2 insertions(+), 0 deletions(-)
Yes, you're right. (When I tried this, checkout complained about the
dirty working tree because a *merge* was needed.)
So let's try to explicitly reset the index and work tree:
$ git reset --hard master
$ vi foo
$ git commit -a -m 'fixed typos'
Oops, still a corrupt commit.
>>>> Another approach is for pull, checkout etc. to automatically update the
>>>> submodule' head ref, but no more.
>>> Then everything, including "git submodule update", would assume
>>> that the submodule is up-to-date.
>> With that approach, "git submodule update" would fetch the submodule's
>> head commit (which could be missing), and then check it against the
>> submodule's index (and maybe its work tree).
> And how is anyone supposed to figure out what HEAD the submodule's
> index and working tree correspond to?
What HEAD corresponds to any other dirty index or dirty working tree?
It's irrelevant and may not exist. You just have some random dirty state.
If it's the yet-to-exist submodule merging you're worried about, the
submodule's old head can be saved in ORIG_HEAD or some such during the
supermodule checkout.
> I can only hope that "git submodule update" would never blindly assume
> that the submodule is clean and so the user would have to manually
> sync the HEAD and the working tree.
Why would it assume that? In this approach, and ignoring submodule
merging for now, "git submodule update" should mean roughly "cd
submodule && git fetch HEAD && git reset --hard HEAD". After all, this
is really the only way to end up with the prescribed commit sha1.
I agree that for safety it makes sense to warn or abort if the index
doesn't match ORIG_HEAD (saved by the supermodule checkout) or if the
index doesn't match the work tree.
Eran
^ permalink raw reply
* Re: [PATCH] (Really) Fix install-doc-quick target
From: Junio C Hamano @ 2007-08-07 3:53 UTC (permalink / raw)
To: Mark Levedahl
Cc: Johannes Schindelin, Mark Levedahl, Git Mailing List, Ren Scharfe
In-Reply-To: <46B7D108.20606@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> + printf "$mandir/%s\n" $(git ls-tree -r --name-only $head) | xargs
> gzip -f
No risk that ls-tree output is too long to fit within the exec
args limit to run printf?
^ permalink raw reply
* Re: [PATCH] git checkout's reflog: even when detaching the HEAD, say from where
From: Junio C Hamano @ 2007-08-07 3:56 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0708070242140.14781@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> @@ -272,7 +272,8 @@ if [ "$?" -eq 0 ]; then
> fi
> elif test -n "$detached"
> then
> - git update-ref --no-deref -m "checkout: moving to $arg" HEAD "$detached" ||
> + old_branch_name=`expr "z$oldbranch" : 'zrefs/heads/\(.*\)'`
> + git update-ref --no-deref -m "checkout: moving from $old_branch_name to $arg" HEAD "$detached" ||
Can't old_branch_name be empty here if you are already detached?
> die "Cannot detach HEAD"
> if test -n "$detach_warn"
> then
> --
> 1.5.3.rc4.17.gb980
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: Linus Torvalds @ 2007-08-07 4:22 UTC (permalink / raw)
To: Steven Grimm
Cc: Johannes Schindelin, Jean-Fran?ois Veillette, Matthieu Moy,
Junio C Hamano, git
In-Reply-To: <20070803053717.GA16379@midwinter.com>
On Thu, 2 Aug 2007, Steven Grimm wrote:
>
> The default is now to not show the diff --git header line if the file's
> timestamp has changed but the contents and/or file mode haven't.
I don't mind this per se, but I'd *really* want some kind of warning that
the index is not up-to-date.
Otherwise, git usage can be horrendously slow, and you're never even told
why. The diffs just take lots of time (because it reads each file), but
the output is empty.
> Personally I'm in favor of doing away with the option altogether
> and having the code always work the way it works by default with
> this patch, but if some people find the old behavior useful they
> can still get at it with the new option.
It's not that the old output is "useful" in itself, but it's important for
people to know that the index is clean. So I'd suggest just setting a flag
when the header isn't printed, and then printing out a single line at the
end about "git index not up-to-date" or something.
Doing a "git diff" cannot actually update the index (since it very much
has to work on a read-only setup too), which is why the index _stays_
stale unless something is done (eg "git status") to refresh it. And it's
that stale index that continues to make for bad performance without any
indication of why that is a problem.
Linus
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: Junio C Hamano @ 2007-08-07 4:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Steven Grimm, Johannes Schindelin, Jean-Fran?ois Veillette,
Matthieu Moy, git
In-Reply-To: <alpine.LFD.0.999.0708062118190.5037@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> It's not that the old output is "useful" in itself, but it's important for
> people to know that the index is clean. So I'd suggest just setting a flag
> when the header isn't printed, and then printing out a single line at the
> end about "git index not up-to-date" or something.
That's essentially the patch I sent out in another thread allows
you to do, and some of these people even Acked them, but there
is one minor issue. "git diff" output is paged, and that "not
up to date" warning, if it is given to stderr, would not be
usually seen.
> Doing a "git diff" cannot actually update the index (since it very much
> has to work on a read-only setup too), which is why the index _stays_
> stale unless something is done (eg "git status") to refresh it. And it's
> that stale index that continues to make for bad performance without any
> indication of why that is a problem.
Indeed.
At least, I am now glad to know that somebody else is of the
same opinion as I am.
^ permalink raw reply
* git-p4.py: doesn't handle symlinks
From: Brian Swetland @ 2007-08-07 4:59 UTC (permalink / raw)
To: git
My python-fu is weak and I don't have the time tonight to unravel it,
but in case somebody cares (or has a quick fix), it appears that the
git-p4.py script deals with symlinks in p4 by checking in a textfile
(to git) containing the name of the target of the symlink.
Of course there *shouldn't* be symlinks in the p4 depot I'm importing
from at all, but that's a different sort of problem.
Brian
^ permalink raw reply
* Re: Problem with bisect
From: Christian Couder @ 2007-08-07 5:26 UTC (permalink / raw)
To: Larry Finger; +Cc: git
In-Reply-To: <46B7DEB5.4060402@lwfinger.net>
Le mardi 7 août 2007 04:53, Larry Finger a écrit :
>
> Has the ability to use a commit hash to indicate a start point been in
> git for a long time?
Yes, it has always been there.
> I think I remember trying it before when a version
> tag had not been downloaded and having a failure.
It should have worked.
Best regards,
Christian.
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: Steven Grimm @ 2007-08-07 5:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jean-Fran?ois Veillette, Matthieu Moy,
Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999.0708062118190.5037@woody.linux-foundation.org>
Linus Torvalds wrote:
> It's not that the old output is "useful" in itself, but it's important for
> people to know that the index is clean. So I'd suggest just setting a flag
> when the header isn't printed, and then printing out a single line at the
> end about "git index not up-to-date" or something.
>
Or even a count of the number of files whose index data is unclean. I'd
be fine with that as a suffix to the diff output.
> Doing a "git diff" cannot actually update the index (since it very much
> has to work on a read-only setup too), which is why the index _stays_
> stale unless something is done (eg "git status") to refresh it. And it's
> that stale index that continues to make for bad performance without any
> indication of why that is a problem.
>
I totally agree that there needs to be a way to tell if the index is
clean or not. I do wonder if the default output of "git diff" is the
right place for that information, but if the notification can be
collapsed to a line or two (rather than the unbounded number of lines
that it potentially outputs now) then that's probably good enough.
Actually, though this will probably make people roll their eyes, before
this discussion I would have guessed that "git status" would be the
command that would tell you the index was out of date, and that there'd
be a separate command (say, "git update-index"?) that you could then use
to sync things up again. The fact that "git status" is really "git
update index a little bit then show status" was not something I
expected; it presents itself as a query utility, not an update utility,
so I would have expected it to be read-only. Its index-modifying
behavior is not even hinted at in the documentation (a patch for which
follows.)
-Steve
^ permalink raw reply
* [PATCH] Add a note about the index being updated by git-status in some cases
From: Steven Grimm @ 2007-08-07 5:57 UTC (permalink / raw)
To: git
In-Reply-To: <46B80993.3080409@midwinter.com>
Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
Documentation/git-status.txt | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6f16eb0..8fd0fc6 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -27,6 +27,13 @@ The command takes the same set of options as `git-commit`; it
shows what would be committed if the same options are given to
`git-commit`.
+If any paths have been touched in the working tree (that is,
+their modification times have changed) but their contents and
+permissions are identical to those in the index file, the command
+updates the index file. Running `git-status` can thus speed up
+subsequent operations such as `git-diff` if the working tree
+contains many paths that have been touched but not modified.
+
OUTPUT
------
--
1.5.3.rc2.4.g726f9
^ permalink raw reply related
* What's in git.git (stable)
From: Junio C Hamano @ 2007-08-07 6:22 UTC (permalink / raw)
To: git
In-Reply-To: <7vvec4synj.fsf_-_@assigned-by-dhcp.cox.net>
* The 'maint' branch has these fixes since the last
announcement, which are all included in 'master' as well.
Because 1.5.3 is just around the corner, I think it is
pointless to do 1.5.2.5 from here, though.
Christian Couder (1):
rev-list --bisect: fix allocation of "int*" instead of "int".
Junio C Hamano (1):
setup.c:verify_non_filename(): don't die unnecessarily while
disambiguating
Linus Torvalds (1):
apply: remove directory that becomes empty by renaming the last
file away
* The 'master' branch has these since 1.5.3-rc4.
Adam Roben (1):
Documentation/git-svn: how to clone a git-svn-created repository
Gerrit Pape (1):
git-am: initialize variable $resume on startup
J. Bruce Fields (4):
user-manual: update for new default --track behavior
user-manual: mention git-gui
documentation: use the word "index" in the git-add manual page
documentation: use the word "index" in the git-commit man page
Jakub Narebski (1):
gitweb: Fix handling of $file_name in feed generation
Johannes Schindelin (1):
checkout-index needs a working tree
Junio C Hamano (8):
git-completion: add "git stash"
INSTALL: add warning on docbook-xsl 1.72 and 1.73
unpack-trees.c: assume submodules are clean during check-out
Fix install-doc-quick target
user-manual: mention git stash
setup.c:verify_non_filename(): don't die unnecessarily while
disambiguating
pager: find out pager setting from configuration
Fix "make GZ=1 quick-install-doc"
Jyotirmoy Bhattacharya (1):
Fixed git-push manpage
Linus Torvalds (1):
apply: remove directory that becomes empty by renaming the last
file away
Randal L. Schwartz (1):
add "test-absolute-path" to .gitignore
Shawn O. Pearce (1):
Document GIT_SSH environment variable alongside other variables
Uwe Kleine-König (1):
send-email: teach sanitize_address to do rfc2047 quoting
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: David Kastrup @ 2007-08-07 6:32 UTC (permalink / raw)
To: git
In-Reply-To: <7vodhkbdx2.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Unfortunately, the patch solves the "large and irrelevant output"
>> of git-diff, but not the performance problem (see the rest of the
>> thread, I failed to convince Junio that updating the index was a
>> performance improvement while keeping the same user semantics).
>
> That's what update-index --refresh (or status if you insist) are
> for, and the coalmine canary you are so dead set to kill are helping
> you realize the need for running.
That does not convince me. Cache staleness should be a problem of
git, not of the user. In particular if the user is just using
porcelain. If letting the cache get stale impacts performance, then
git should clean up its act on its own without barfing when using
unrelated commands. If it notices this during diff (presumably by
overstepping some staleness ratio), then it can set a "regenerate on
next opportunity" flag on the index, and then the next command wanting
to process the index from the start can rewrite a refreshed version.
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] Documentation/Makefile: remove cmd-list.made before redirecting to it.
From: Junio C Hamano @ 2007-08-07 6:33 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <86vebsby27.fsf@lola.quinscape.zz>
David Kastrup <dak@gnu.org> writes:
> If cmd-list.made has been created by a previous run as root, output
> redirection to it will fail. So remove it before regeneration.
>
> Signed-off-by: David Kastrup <dak@gnu.org>
> ---
> Documentation/Makefile | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 97ee067..120e7c0 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -120,6 +120,7 @@ $(cmds_txt): cmd-list.made
>
> cmd-list.made: cmd-list.perl $(MAN1_TXT)
> perl ./cmd-list.perl
> + $(RM) $@
> date >$@
>
> git.7 git.html: git.txt core-intro.txt
Although I understand that it would be a problem if you built as
root earlier, which would have left files unmodifyable by you, I
think this is getting out of hand. The cmd-list.perl script
itself, for example, does "creat in $out+, if the contents have
changed from the last round then rename $out+ to $out" sequence
in order to avoid unnecessary rebuild of files that depend on
the generated command list. If it is interrupted in the middle
while running as root, and then you try to do another build, I
suspect "creat in $out+" part would fail.
Maybe you can simply recover from such an error with a "make
clean"?
Also I'd prefer $(RM) before actually running the command to
generate the list, but that is just the matter of taste.
^ permalink raw reply
* [PATCH] git-diff: Output a warning about stale files in the index
From: Steven Grimm @ 2007-08-07 6:35 UTC (permalink / raw)
To: git
In-Reply-To: <46B80993.3080409@midwinter.com>
Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
This is based on (and includes) Junio's patch. This should
hopefully address the "I want to know when my index is very
stale" problem with both his original patch and mine.
If we are running a pager, I output the warning to standard
output so it doesn't get immediately scrolled off the screen by
the paged diff output. Otherwise I output to standard error
which is really the more appropriate place for the warning.
Obviously that is no good if the user is running his own pager,
but I'm not sure how to detect that and not cause problems for
diffs that are piped into other programs.
diff.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
diffcore.h | 1 +
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index a5fc56b..7b11195 100644
--- a/diff.c
+++ b/diff.c
@@ -2979,7 +2979,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
free(q->queue);
q->queue = NULL;
- q->nr = q->alloc = 0;
+ q->nr = q->alloc = q->removed = 0;
return result;
}
@@ -3015,6 +3015,17 @@ void diff_flush(struct diff_options *options)
int i, output_format = options->output_format;
int separator = 0;
+ if (q->removed > 0 && ! (output_format & DIFF_FORMAT_NO_OUTPUT)) {
+ char *format = "Warning: %d %s touched but not modified. "
+ "Consider running git-status.\n";
+ char *plural = q->removed == 1 ? "path" : "paths";
+
+ if (pager_in_use)
+ printf(format, q->removed, plural);
+ else
+ fprintf(stderr, format, q->removed, plural);
+ }
+
/*
* Order: raw, stat, summary, patch
* or: name/name-status/checkdiff (other bits clear)
@@ -3084,7 +3095,7 @@ void diff_flush(struct diff_options *options)
free_queue:
free(q->queue);
q->queue = NULL;
- q->nr = q->alloc = 0;
+ q->nr = q->alloc = q->removed = 0;
}
static void diffcore_apply_filter(const char *filter)
@@ -3093,7 +3104,7 @@ static void diffcore_apply_filter(const char *filter)
struct diff_queue_struct *q = &diff_queued_diff;
struct diff_queue_struct outq;
outq.queue = NULL;
- outq.nr = outq.alloc = 0;
+ outq.nr = outq.alloc = outq.removed = 0;
if (!filter)
return;
@@ -3143,6 +3154,47 @@ static void diffcore_apply_filter(const char *filter)
*q = outq;
}
+static void diffcore_remove_empty(void)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+ struct diff_queue_struct outq;
+ outq.queue = NULL;
+ outq.nr = outq.alloc = outq.removed = 0;
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+
+ /*
+ * 1. Keep the ones that cannot be diff-files
+ * "false" match that are only queued due to
+ * cache dirtyness.
+ *
+ * 2. Modified, same size and mode, and the object
+ * name of one side is unknown. If they do not
+ * have identical contents, keep them.
+ * They are different.
+ */
+ if ((p->status != DIFF_STATUS_MODIFIED) || /* (1) */
+ (p->one->sha1_valid && p->two->sha1_valid) ||
+ (p->one->mode != p->two->mode) ||
+
+ diff_populate_filespec(p->one, 1) || /* (2) */
+ diff_populate_filespec(p->two, 1) ||
+ (p->one->size != p->two->size) ||
+ diff_populate_filespec(p->one, 0) ||
+ diff_populate_filespec(p->two, 0) ||
+ memcmp(p->one->data, p->two->data, p->one->size))
+ diff_q(&outq, p);
+ else {
+ diff_free_filepair(p);
+ outq.removed++;
+ }
+ }
+ free(q->queue);
+ *q = outq;
+}
+
void diffcore_std(struct diff_options *options)
{
if (options->quiet)
@@ -3160,6 +3212,7 @@ void diffcore_std(struct diff_options *options)
diffcore_order(options->orderfile);
diff_resolve_rename_copy();
diffcore_apply_filter(options->filter);
+ diffcore_remove_empty();
options->has_changes = !!diff_queued_diff.nr;
}
diff --git a/diffcore.h b/diffcore.h
index eef17c4..e5a9244 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -81,6 +81,7 @@ struct diff_queue_struct {
struct diff_filepair **queue;
int alloc;
int nr;
+ int removed;
};
extern struct diff_queue_struct diff_queued_diff;
--
1.5.3.rc2.4.g726f9
^ permalink raw reply related
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: Steven Grimm @ 2007-08-07 6:39 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jean-Fran?ois Veillette, Matthieu Moy,
Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999.0708062118190.5037@woody.linux-foundation.org>
Linus Torvalds wrote:
> Doing a "git diff" cannot actually update the index (since it very much
> has to work on a read-only setup too), which is why the index _stays_
> stale unless something is done (eg "git status") to refresh it.
Another thought: How about if git-diff *tries* to update the index if
needed, but failure to do so is not treated as an error condition? That
seems like the best of both worlds to me: git would self-correct a
potential performance problem without user intervention, while still
working properly in a read-only environment.
-Steve
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: David Kastrup @ 2007-08-07 6:41 UTC (permalink / raw)
To: git
In-Reply-To: <7v4pjc9czm.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> writes:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Doing a "git diff" cannot actually update the index (since it very
>> much has to work on a read-only setup too), which is why the index
>> _stays_ stale unless something is done (eg "git status") to refresh
>> it. And it's that stale index that continues to make for bad
>> performance without any indication of why that is a problem.
>
> Indeed.
>
> At least, I am now glad to know that somebody else is of the same
> opinion as I am.
I don't want a system to tell me when it is shooting itself in the
foot. It should not be doing this in the first place.
File systems have automatic fsck procedures enforced regularly, too,
to keep them operative. If git finds that it is getting inefficient,
it should just mark the index as "regenerate at next access". And
then do it.
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] git-diff: Output a warning about stale files in the index
From: Junio C Hamano @ 2007-08-07 6:46 UTC (permalink / raw)
To: Steven Grimm; +Cc: git
In-Reply-To: <20070807063523.GA29617@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> Signed-off-by: Steven Grimm <koreth@midwinter.com>
> ---
> This is based on (and includes) Junio's patch. This should
> hopefully address the "I want to know when my index is very
> stale" problem with both his original patch and mine.
>
> If we are running a pager, I output the warning to standard
> output so it doesn't get immediately scrolled off the screen by
> the paged diff output. Otherwise I output to standard error
> which is really the more appropriate place for the warning.
> Obviously that is no good if the user is running his own pager,
> but I'm not sure how to detect that and not cause problems for
> diffs that are piped into other programs.
Hmph. One way to avoid causing problems for diffs that are
piped into other programs and still give the "index of sync"
warning is to emit "diff --git" line and no patch body fot
textual diffs, or 0{40} SHA-1 on the right hand side for --raw
format diffs.
Jokes aside...
For textual diffs, I think we can always spit out the warning
message at the beginning of at the end on the standard output
without harming any of the patch based toolchain.
So how about...
- If and only if the output format asks for textual diff
(DIFF_FORMAT_PATCH), we do this "stat-dirty-removal";
otherwise we do not spend extra cycles and keep the current
behaviour.
- At the end of patch text, show "stat-dirty-removal" warning
on stdout.
^ permalink raw reply
* Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged
From: Matthieu Moy @ 2007-08-07 6:47 UTC (permalink / raw)
To: Linus Torvalds
Cc: Steven Grimm, Johannes Schindelin, Jean-Fran?ois Veillette,
Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999.0708062118190.5037@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> It's not that the old output is "useful" in itself, but it's important for
> people to know that the index is clean. So I'd suggest just setting a flag
> when the header isn't printed, and then printing out a single line at the
> end about "git index not up-to-date" or something.
Yes. Junio's patch has this as a comment, it's probably good to
uncomment it, and perhaps print it directly on stdout so that you see
it even with a pager.
> Doing a "git diff" cannot actually update the index (since it very much
> has to work on a read-only setup too),
Err, what's the relationship between the two parts of your sentence?
You can't be sure that git-diff will update the index (because you may
be working on a read-only setup, yes), but git-diff can at least _try_
to, and fall-back to the read-only behavior if updating the index
fails.
That's not a highly original idea since this is what git already does
with "status".
Once more, I'm willing to write the code for that if it has a chance
to be accepted.
--
Matthieu
^ permalink raw reply
* [PATCH v2] git-diff: Output a warning about stale files in the index
From: Steven Grimm @ 2007-08-07 7:17 UTC (permalink / raw)
To: git
In-Reply-To: <7vbqdj9709.fsf@assigned-by-dhcp.cox.net>
Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
Modified as suggested by Junio.
diff.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
diffcore.h | 1 +
2 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index a5fc56b..5f2e1fe 100644
--- a/diff.c
+++ b/diff.c
@@ -2979,7 +2979,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
free(q->queue);
q->queue = NULL;
- q->nr = q->alloc = 0;
+ q->nr = q->alloc = q->removed = 0;
return result;
}
@@ -3074,6 +3074,12 @@ void diff_flush(struct diff_options *options)
if (check_pair_status(p))
diff_flush_patch(p, options);
}
+
+ if (q->removed > 0) {
+ printf("Warning: %d %s touched but not modified. "
+ "Consider running git-status.\n",
+ q->removed, q->removed == 1 ? "path" : "paths");
+ }
}
if (output_format & DIFF_FORMAT_CALLBACK)
@@ -3084,7 +3090,7 @@ void diff_flush(struct diff_options *options)
free_queue:
free(q->queue);
q->queue = NULL;
- q->nr = q->alloc = 0;
+ q->nr = q->alloc = q->removed = 0;
}
static void diffcore_apply_filter(const char *filter)
@@ -3093,7 +3099,7 @@ static void diffcore_apply_filter(const char *filter)
struct diff_queue_struct *q = &diff_queued_diff;
struct diff_queue_struct outq;
outq.queue = NULL;
- outq.nr = outq.alloc = 0;
+ outq.nr = outq.alloc = outq.removed = 0;
if (!filter)
return;
@@ -3143,6 +3149,47 @@ static void diffcore_apply_filter(const char *filter)
*q = outq;
}
+static void diffcore_remove_empty(void)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+ struct diff_queue_struct outq;
+ outq.queue = NULL;
+ outq.nr = outq.alloc = outq.removed = 0;
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+
+ /*
+ * 1. Keep the ones that cannot be diff-files
+ * "false" match that are only queued due to
+ * cache dirtyness.
+ *
+ * 2. Modified, same size and mode, and the object
+ * name of one side is unknown. If they do not
+ * have identical contents, keep them.
+ * They are different.
+ */
+ if ((p->status != DIFF_STATUS_MODIFIED) || /* (1) */
+ (p->one->sha1_valid && p->two->sha1_valid) ||
+ (p->one->mode != p->two->mode) ||
+
+ diff_populate_filespec(p->one, 1) || /* (2) */
+ diff_populate_filespec(p->two, 1) ||
+ (p->one->size != p->two->size) ||
+ diff_populate_filespec(p->one, 0) ||
+ diff_populate_filespec(p->two, 0) ||
+ memcmp(p->one->data, p->two->data, p->one->size))
+ diff_q(&outq, p);
+ else {
+ diff_free_filepair(p);
+ outq.removed++;
+ }
+ }
+ free(q->queue);
+ *q = outq;
+}
+
void diffcore_std(struct diff_options *options)
{
if (options->quiet)
@@ -3160,6 +3207,8 @@ void diffcore_std(struct diff_options *options)
diffcore_order(options->orderfile);
diff_resolve_rename_copy();
diffcore_apply_filter(options->filter);
+ if (options->output_format & DIFF_FORMAT_PATCH)
+ diffcore_remove_empty();
options->has_changes = !!diff_queued_diff.nr;
}
diff --git a/diffcore.h b/diffcore.h
index eef17c4..e5a9244 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -81,6 +81,7 @@ struct diff_queue_struct {
struct diff_filepair **queue;
int alloc;
int nr;
+ int removed;
};
extern struct diff_queue_struct diff_queued_diff;
--
1.5.3.rc2.4.g726f9
^ permalink raw reply related
* Re: [PATCH v2] git-diff: Output a warning about stale files in the index
From: Junio C Hamano @ 2007-08-07 7:46 UTC (permalink / raw)
To: Steven Grimm; +Cc: git
In-Reply-To: <20070807071712.GA32751@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> Signed-off-by: Steven Grimm <koreth@midwinter.com>
> ---
> Modified as suggested by Junio.
Okay.
Assuming that other people are happy with this version (I have
to warn you that I haven't even attempted to apply this patch,
let alone compiling yet), I'd prefer to keep our combined
thought process in the commit log, so that we do not have to
rehash this later, over and over again.
Something along the following lines, perhaps...?
After starting to edit a working tree file but later when your
edit ends up identical to the original (this can also happen
when you ran a wholesale regexp replace with something like
"perl -i" that does not touch many of the paths), "git diff"
between the index and the working tree outputs many "empty"
diffs that show "diff --git" header and nothing else, because
these paths are stat dirty. While it was _a_ way to warn the
user that the earlier action of the user made the index
ineffective as an optimization mechanism, it was felt too loud
for the purpose of warning even to experienced users, and also
resulted in confusing people new to git.
This replaces the "empty" diffs with a single warning message
at the end. When you see such a message, you know you did
something suboptimal to your index; you can optimize the index
again by running "git-update-index --refresh".
The change affects only "git diff" that outputs patch text,
because that is where the annoyance of too many "empty" diff
is most strongly felt, and because the warning message can be
safely ignored by downstream tools without getting mistaken as
part of the patch. For the low-level "git diff-files", the
traditional behaviour is retained.
^ permalink raw reply
* Re: [PATCH v2] git-diff: Output a warning about stale files in the index
From: Steven Grimm @ 2007-08-07 7:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqdj7poi.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Something along the following lines, perhaps...?
>
That seems like a fine commit message to me. My one-liner definitely
assumed too much context, coming in the middle of this discussion as it
did -- much better to be explicit about the reasoning for posterity's sake.
-Steve
^ permalink raw reply
* [PATCH] git-p4: Fix support for symlinks.
From: Simon Hausmann @ 2007-08-07 8:25 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Brian Swetland
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
Signed-off-by: Simon Hausmann <simon@lst.de>
---
contrib/fast-import/git-p4 | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 41e86e7..9c6f911 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -839,11 +839,15 @@ class P4Sync(Command):
if file["action"] == "delete":
self.gitStream.write("D %s\n" % relPath)
else:
+ data = file['data']
+
mode = 644
if file["type"].startswith("x"):
mode = 755
-
- data = file['data']
+ elif file["type"] == "symlink":
+ mode = 120000
+ # p4 print on a symlink contains "target\n", so strip it off
+ data = data[:-1]
if self.isWindows and file["type"].endswith("text"):
data = data.replace("\r\n", "\n")
--
1.5.3.rc3.91.g5c75
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: [PATCH] git-p4: Fix support for symlinks.
From: Junio C Hamano @ 2007-08-07 8:40 UTC (permalink / raw)
To: Simon Hausmann; +Cc: git, Han-Wen Nienhuys, Brian Swetland
In-Reply-To: <200708071025.47965.simon@lst.de>
Simon Hausmann <simon@lst.de> writes:
> Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
>
> Signed-off-by: Simon Hausmann <simon@lst.de>
> ---
> contrib/fast-import/git-p4 | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 41e86e7..9c6f911 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -839,11 +839,15 @@ class P4Sync(Command):
> if file["action"] == "delete":
> self.gitStream.write("D %s\n" % relPath)
> else:
> + data = file['data']
> +
> mode = 644
> if file["type"].startswith("x"):
> mode = 755
> -
> - data = file['data']
> + elif file["type"] == "symlink":
> + mode = 120000
> + # p4 print on a symlink contains "target\n", so strip it off
> + data = data[:-1]
>
> if self.isWindows and file["type"].endswith("text"):
> data = data.replace("\r\n", "\n")
Thanks for a quick fix.
Brian, does this resolve the issue for you? I do not have an
access to p4 myself so I won't make a good judge in this area
myself. An Ack is appreciated.
Simon, just a style nit.
Every time I see decimal integers 644 and/or 755, it interrupts
my flow of thought and forces me to read the change and its
surrounding text needlessly carefully.
If you read the code, you can see that this "mode" variable is
formatted with ("%d" % mode) for writing out, and is not used as
permission bit pattern in any bitwise operations, so you can
tell that these constants _are_ safe. But it takes extra
efforts to convince yourself that they indeed are.
I would prefer this kind of thing to be written as either:
mode = 0644
"%o" % mode
or
mode = "644"
"%s" % mode
to make it clear that the author knew what he was doing when he
wrote the code.
^ permalink raw reply
* Re: [PATCH] unpack-trees.c: assume submodules are clean during check-out
From: Sven Verdoolaege @ 2007-08-07 8:51 UTC (permalink / raw)
To: Eran Tromer; +Cc: Junio C Hamano, git
In-Reply-To: <46B7E5FE.7050006@tromer.org>
On Mon, Aug 06, 2007 at 11:24:46PM -0400, Eran Tromer wrote:
> On 2007-08-06 15:03, Sven Verdoolaege wrote:
> >>>> Another approach is for pull, checkout etc. to automatically update the
> >>>> submodule' head ref, but no more.
> >>> Then everything, including "git submodule update", would assume
> >>> that the submodule is up-to-date.
> >> With that approach, "git submodule update" would fetch the submodule's
> >> head commit (which could be missing), and then check it against the
> >> submodule's index (and maybe its work tree).
> > And how is anyone supposed to figure out what HEAD the submodule's
> > index and working tree correspond to?
>
> What HEAD corresponds to any other dirty index or dirty working tree?
> It's irrelevant and may not exist. You just have some random dirty state.
The only way to know that it's dirty is if you know the HEAD.
How can that not be relevant.
> > I can only hope that "git submodule update" would never blindly assume
> > that the submodule is clean and so the user would have to manually
> > sync the HEAD and the working tree.
>
> Why would it assume that? In this approach, and ignoring submodule
> merging for now, "git submodule update" should mean roughly "cd
> submodule && git fetch HEAD && git reset --hard HEAD".
If you're doing that, then that is exactly what you are assuming.
> After all, this
> is really the only way to end up with the prescribed commit sha1.
That's the best way of losing all you precious changes in the submodule.
And there is no way to get them back!
Surely this is a lot worse than occasionally committing something you
didn't plan to commit, and only if you are performing a known "dangerous"
operation.
> I agree that for safety it makes sense to warn or abort if the index
> doesn't match ORIG_HEAD (saved by the supermodule checkout) or if the
> index doesn't match the work tree.
You may have done several supermodule checkouts since you last changed
the submodule.
skimo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox