Git development
 help / color / mirror / Atom feed
* [PATCH 11/14] git p4: fail gracefully on sync with no master branch
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

If --branch was used to build a repository with no
refs/remotes/p4/master, future syncs will not know
which branch to sync.  Notice this situation and
print a helpful error message.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py                 | 29 +++++++++++++++++++++++++++--
 t/t9806-git-p4-options.sh |  9 ++++-----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 9b07ddd..390d3f1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -585,6 +585,17 @@ def p4BranchesInGit(branchesAreInRemotes=True):
 
     return branches
 
+def branch_exists(branch):
+    """Make sure that the given ref name really exists."""
+
+    cmd = [ "git", "rev-parse", "--symbolic", "--verify", branch ]
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    out, _ = p.communicate()
+    if p.returncode:
+        return False
+    # expect exactly one line of output: the branch name
+    return out.rstrip() == branch
+
 def findUpstreamBranchPoint(head = "HEAD"):
     branches = p4BranchesInGit()
     # map from depot-path to branch name
@@ -2774,6 +2785,7 @@ class P4Sync(Command, P4UserMap):
                     print 'Syncing with origin first, using "git fetch origin"'
                 system("git fetch origin")
 
+        branch_arg_given = bool(self.branch)
         if len(self.branch) == 0:
             self.branch = self.refPrefix + "master"
             if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
@@ -2967,8 +2979,21 @@ class P4Sync(Command, P4UserMap):
             else:
                 # catch "git p4 sync" with no new branches, in a repo that
                 # does not have any existing p4 branches
-                if len(args) == 0 and not self.p4BranchesInGit:
-                    die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.");
+                if len(args) == 0:
+                    if not self.p4BranchesInGit:
+                        die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.")
+
+                    # The default branch is master, unless --branch is used to
+                    # specify something else.  Make sure it exists, or complain
+                    # nicely about how to use --branch.
+                    if not self.detectBranches:
+                        if not branch_exists(self.branch):
+                            if branch_arg_given:
+                                die("Error: branch %s does not exist." % self.branch)
+                            else:
+                                die("Error: no branch %s; perhaps specify one with --branch." %
+                                    self.branch)
+
                 if self.verbose:
                     print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths),
                                                               self.changeRange)
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index c0d4433..a51f122 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -40,14 +40,13 @@ test_expect_success 'clone --branch should checkout master' '
 	)
 '
 
-test_expect_failure 'sync when branch is not called master should work' '
-	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot@2 &&
+test_expect_success 'sync when no master branch prints a nice error' '
 	test_when_finished cleanup_git &&
+	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot@2 &&
 	(
 		cd "$git" &&
-		git p4 sync &&
-		git show -s --format=%s refs/remotes/p4/sb >show &&
-		grep "change 3" show
+		test_must_fail git p4 sync 2>err &&
+		grep "Error: no branch refs/remotes/p4/master" err
 	)
 '
 
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 10/14] git p4: rearrange self.initialParent use
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

This was set in a couple of places, both of which were very
far away from its use.  Move it a bit closer to importChanges(),
and add some comments.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 5dcb527..9b07ddd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2689,6 +2689,7 @@ class P4Sync(Command, P4UserMap):
                     files = self.extractFilesFromCommit(description)
                     self.commit(description, files, self.branch,
                                 self.initialParent)
+                    # only needed once, to connect to the previous commit
                     self.initialParent = ""
             except IOError:
                 print self.gitError.read()
@@ -2754,7 +2755,6 @@ class P4Sync(Command, P4UserMap):
     def run(self, args):
         self.depotPaths = []
         self.changeRange = ""
-        self.initialParent = ""
         self.previousDepotPaths = []
         self.hasOrigin = False
 
@@ -2842,8 +2842,6 @@ class P4Sync(Command, P4UserMap):
             if p4Change > 0:
                 self.depotPaths = sorted(self.previousDepotPaths)
                 self.changeRange = "@%s,#head" % p4Change
-                if not self.detectBranches:
-                    self.initialParent = parseRevision(self.branch)
                 if not self.silent and not self.detectBranches:
                     print "Performing incremental import into %s git branch" % self.branch
 
@@ -2988,6 +2986,14 @@ class P4Sync(Command, P4UserMap):
 
                 self.updatedBranches = set()
 
+                if not self.detectBranches:
+                    if args:
+                        # start a new branch
+                        self.initialParent = ""
+                    else:
+                        # build on a previous revision
+                        self.initialParent = parseRevision(self.branch)
+
                 self.importChanges(changes)
 
                 if not self.silent:
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 09/14] git p4: allow short ref names to --branch
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

For a clone or sync, --branch says where the newly imported
branch should go, or which existing branch to sync up.  It
takes an argument, which is currently either something that
starts with "refs/", or if not, "refs/heads/p4" is prepended.

Putting it in heads seems like a bad default; these should
go in remotes/p4/ in most situations.  Make that the new default,
and be more liberal in the form of the branch name.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt  |  7 +++++--
 git-p4.py                 | 12 +++++++++++-
 t/t9806-git-p4-options.sh | 21 +++++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 7c5230e..7bd5c29 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -173,8 +173,11 @@ subsequent 'sync' operations.
 
 --branch <branch>::
 	Import changes into given branch.  If the branch starts with
-	'refs/', it will be used as is, otherwise the path 'refs/heads/'
-	will be prepended.  The default branch is 'p4/master'.
+	'refs/', it will be used as is.  Otherwise if it does not start
+	with 'p4/', that prefix is added.  The branch is assumed to
+	name a remote tracking, but this can be modified using
+	'--import-local', or by giving a full ref name.  The default
+	branch is 'master'.
 +
 This example imports a new remote "p4/proj2" into an existing
 git repository:
diff --git a/git-p4.py b/git-p4.py
index d92f00c..5dcb527 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2847,8 +2847,18 @@ class P4Sync(Command, P4UserMap):
                 if not self.silent and not self.detectBranches:
                     print "Performing incremental import into %s git branch" % self.branch
 
+        # accept multiple ref name abbreviations:
+        #    refs/foo/bar/branch -> use it exactly
+        #    p4/branch -> prepend refs/remotes/ or refs/heads/
+        #    branch -> prepend refs/remotes/p4/ or refs/heads/p4/
         if not self.branch.startswith("refs/"):
-            self.branch = "refs/heads/" + self.branch
+            if self.importIntoRemotes:
+                prepend = "refs/remotes/"
+            else:
+                prepend = "refs/heads/"
+            if not self.branch.startswith("p4/"):
+                prepend += "p4/"
+            self.branch = prepend + self.branch
 
         if len(args) == 0 and self.depotPaths:
             if not self.silent:
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 2ad3a3e..c0d4433 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -51,6 +51,27 @@ test_expect_failure 'sync when branch is not called master should work' '
 	)
 '
 
+test_expect_success 'sync --branch builds the full ref name correctly' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git init &&
+
+		git p4 sync --branch=b1 //depot &&
+		git rev-parse --verify refs/remotes/p4/b1 &&
+		git p4 sync --branch=p4/b2 //depot &&
+		git rev-parse --verify refs/remotes/p4/b2 &&
+
+		git p4 sync --import-local --branch=h1 //depot &&
+		git rev-parse --verify refs/heads/p4/h1 &&
+		git p4 sync --import-local --branch=p4/h2 //depot &&
+		git rev-parse --verify refs/heads/p4/h2 &&
+
+		git p4 sync --branch=refs/stuff //depot &&
+		git rev-parse --verify refs/stuff
+	)
+'
+
 # engages --detect-branches code, which will do filename filtering so
 # no sync to either b1 or b2
 test_expect_success 'sync when two branches but no master should noop' '
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 08/14] git p4 doc: fix branch detection example
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

Make sure that the example on how to use git-p4.branchList
works if typed directly.  In particular, it does not make sense
to set a config variable until the git repository has been
initialized.

Reported-by: Olivier Delalleau <shish@keba.be>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 2623bee..7c5230e 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -393,8 +393,10 @@ the path elements in the p4 repository.  The example above relied on the
 presence of the p4 branch.  Without p4 branches, the same result will
 occur with:
 ----
+git init depot
+cd depot
 git config git-p4.branchList main:branch1
-git p4 clone --detect-branches //depot@all
+git p4 clone --detect-branches //depot@all .
 ----
 
 
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 07/14] git p4: clone --branch should checkout master
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

When using the --branch argument to "git p4 clone", one
might specify a destination for p4 changes different from
the default refs/remotes/p4/master.  Both cases should
create a master branch and checkout files.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt  |  3 +--
 git-p4.py                 | 20 +++++++++-----------
 t/t9806-git-p4-options.sh |  2 +-
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index beff622..2623bee 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -174,8 +174,7 @@ subsequent 'sync' operations.
 --branch <branch>::
 	Import changes into given branch.  If the branch starts with
 	'refs/', it will be used as is, otherwise the path 'refs/heads/'
-	will be prepended.  The default branch is 'master'.  If used
-	with an initial clone, no HEAD will be checked out.
+	will be prepended.  The default branch is 'p4/master'.
 +
 This example imports a new remote "p4/proj2" into an existing
 git repository:
diff --git a/git-p4.py b/git-p4.py
index 537eac6..d92f00c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3124,17 +3124,15 @@ class P4Clone(P4Sync):
 
         if not P4Sync.run(self, depotPaths):
             return False
-        if self.branch != "master":
-            if self.importIntoRemotes:
-                masterbranch = "refs/remotes/p4/master"
-            else:
-                masterbranch = "refs/heads/p4/master"
-            if gitBranchExists(masterbranch):
-                system("git branch master %s" % masterbranch)
-                if not self.cloneBare:
-                    system("git checkout -f")
-            else:
-                print "Could not detect main branch. No checkout/master branch created."
+
+        # create a master branch and check out a work tree
+        if gitBranchExists(self.branch):
+            system([ "git", "branch", "master", self.branch ])
+            if not self.cloneBare:
+                system([ "git", "checkout", "-f" ])
+        else:
+            print 'Not checking out any branch, use ' \
+                  '"git checkout -q -b master <branch>"'
 
         # auto-set this variable if invoked with --use-client-spec
         if self.useClientSpec_from_options:
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 4900aef..2ad3a3e 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -27,7 +27,7 @@ test_expect_success 'clone no --git-dir' '
 	test_must_fail git p4 clone --git-dir=xx //depot
 '
 
-test_expect_failure 'clone --branch should checkout master' '
+test_expect_success 'clone --branch should checkout master' '
 	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	(
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 06/14] git p4: verify expected refs in clone --bare test
From: Pete Wyckoff @ 2013-01-15  0:47 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

Make sure that the standard branches are created as expected.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9800-git-p4-basic.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 8c59796..166e752 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -160,9 +160,12 @@ test_expect_success 'clone --bare should make a bare repository' '
 	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
-		test ! -d .git &&
-		bare=`git config --get core.bare` &&
-		test "$bare" = true
+		test_path_is_missing .git &&
+		git config --get --bool core.bare true &&
+		git rev-parse --verify refs/remotes/p4/master &&
+		git rev-parse --verify refs/remotes/p4/HEAD &&
+		git rev-parse --verify refs/heads/master &&
+		git rev-parse --verify HEAD
 	)
 '
 
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 05/14] git p4: create p4/HEAD on initial clone
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

There is code to create a symbolic reference from p4/HEAD to
p4/master.  This allows saying "git show p4" as a shortcut
to "git show p4/master", for example.

But this reference was only created on the second "git p4 sync"
(or first sync after a clone).  Make it work on the initial
clone or sync.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py                 | 12 ++++++++----
 t/t9806-git-p4-options.sh | 23 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 8814049..537eac6 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2778,10 +2778,7 @@ class P4Sync(Command, P4UserMap):
             self.branch = self.refPrefix + "master"
             if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
                 system("git update-ref %s refs/heads/p4" % self.branch)
-                system("git branch -D p4");
-            # create it /after/ importing, when master exists
-            if not gitBranchExists(self.refPrefix + "HEAD") and self.importIntoRemotes and gitBranchExists(self.branch):
-                system("git symbolic-ref %sHEAD %s" % (self.refPrefix, self.branch))
+                system("git branch -D p4")
 
         # accept either the command-line option, or the configuration variable
         if self.useClientSpec:
@@ -3013,6 +3010,13 @@ class P4Sync(Command, P4UserMap):
                 read_pipe("git update-ref -d %s" % branch)
             os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
 
+        # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
+        # a convenient shortcut refname "p4".
+        if self.importIntoRemotes:
+            head_ref = self.refPrefix + "HEAD"
+            if not gitBranchExists(head_ref) and gitBranchExists(self.branch):
+                system(["git", "symbolic-ref", head_ref, self.branch])
+
         return True
 
 class P4Rebase(Command):
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 844aae0..4900aef 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -83,6 +83,29 @@ test_expect_failure 'sync --branch updates specified branch' '
 	)
 '
 
+# allows using the refname "p4" as a short name for p4/master
+test_expect_success 'clone creates HEAD symbolic reference' '
+	git p4 clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git rev-parse --verify refs/remotes/p4/master >master &&
+		git rev-parse --verify p4 >p4 &&
+		test_cmp master p4
+	)
+'
+
+test_expect_success 'clone --branch creates HEAD symbolic reference' '
+	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git rev-parse --verify refs/remotes/p4/sb >sb &&
+		git rev-parse --verify p4 >p4 &&
+		test_cmp sb p4
+	)
+'
+
 test_expect_success 'clone --changesfile' '
 	test_when_finished "rm cf" &&
 	printf "1\n3\n" >cf &&
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 04/14] git p4: inline listExistingP4GitBranches
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

It is four lines of code used in only one place.  Simplify by
including it where it is used.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 03680b0..8814049 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2518,13 +2518,6 @@ class P4Sync(Command, P4UserMap):
                 branch = branch[len(self.projectName):]
             self.knownBranches[branch] = branch
 
-    def listExistingP4GitBranches(self):
-        # branches holds mapping from name to commit
-        branches = p4BranchesInGit(self.importIntoRemotes)
-        self.p4BranchesInGit = branches.keys()
-        for branch in branches.keys():
-            self.initialParents[self.refPrefix + branch] = branches[branch]
-
     def updateOptionDict(self, d):
         option_keys = {}
         if self.keepRepoPath:
@@ -2805,7 +2798,12 @@ class P4Sync(Command, P4UserMap):
         if args == []:
             if self.hasOrigin:
                 createOrUpdateBranchesFromOrigin(self.refPrefix, self.silent)
-            self.listExistingP4GitBranches()
+
+            # branches holds mapping from branch name to sha1
+            branches = p4BranchesInGit(self.importIntoRemotes)
+            self.p4BranchesInGit = branches.keys()
+            for branch in branches.keys():
+                self.initialParents[self.refPrefix + branch] = branches[branch]
 
             if len(self.p4BranchesInGit) > 1:
                 if not self.silent:
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 03/14] git p4: add comments to p4BranchesInGit
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>


Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 68f7458..03680b0 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -553,27 +553,36 @@ def gitConfigList(key):
         _gitConfig[key] = read_pipe("git config --get-all %s" % key, ignore_error=True).strip().split(os.linesep)
     return _gitConfig[key]
 
-def p4BranchesInGit(branchesAreInRemotes = True):
+def p4BranchesInGit(branchesAreInRemotes=True):
+    """Find all the branches whose names start with "p4/", looking
+       in remotes or heads as specified by the argument.  Return
+       a dictionary of { branch: revision } for each one found.
+       The branch names are the short names, without any
+       "p4/" prefix."""
+
     branches = {}
 
     cmdline = "git rev-parse --symbolic "
     if branchesAreInRemotes:
-        cmdline += " --remotes"
+        cmdline += "--remotes"
     else:
-        cmdline += " --branches"
+        cmdline += "--branches"
 
     for line in read_pipe_lines(cmdline):
         line = line.strip()
 
-        ## only import to p4/
-        if not line.startswith('p4/') or line == "p4/HEAD":
+        # only import to p4/
+        if not line.startswith('p4/'):
+            continue
+        # special symbolic ref to p4/master
+        if line == "p4/HEAD":
             continue
-        branch = line
 
-        # strip off p4
-        branch = re.sub ("^p4/", "", line)
+        # strip off p4/ prefix
+        branch = line[len("p4/"):]
 
         branches[branch] = parseRevision(line)
+
     return branches
 
 def findUpstreamBranchPoint(head = "HEAD"):
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 02/14] git p4: rearrange and simplify hasOrigin handling
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>


Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 git-p4.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 69f1452..68f7458 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2754,23 +2754,23 @@ class P4Sync(Command, P4UserMap):
         self.changeRange = ""
         self.initialParent = ""
         self.previousDepotPaths = []
+        self.hasOrigin = False
 
         # map from branch depot path to parent branch
         self.knownBranches = {}
         self.initialParents = {}
-        self.hasOrigin = originP4BranchesExist()
-        if not self.syncWithOrigin:
-            self.hasOrigin = False
 
         if self.importIntoRemotes:
             self.refPrefix = "refs/remotes/p4/"
         else:
             self.refPrefix = "refs/heads/p4/"
 
-        if self.syncWithOrigin and self.hasOrigin:
-            if not self.silent:
-                print "Syncing with origin first by calling git fetch origin"
-            system("git fetch origin")
+        if self.syncWithOrigin:
+            self.hasOrigin = originP4BranchesExist()
+            if self.hasOrigin:
+                if not self.silent:
+                    print 'Syncing with origin first, using "git fetch origin"'
+                system("git fetch origin")
 
         if len(self.branch) == 0:
             self.branch = self.refPrefix + "master"
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 01/14] git p4: test sync/clone --branch behavior
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau
In-Reply-To: <1358210828-2369-1-git-send-email-pw@padd.com>

Add failing tests to document behavior when there are multiple p4
branches, as created using the --branch option.  In particular:

Using clone --branch populates the specified branch correctly, but
dies with an error when trying to checkout master.

Calling sync without a master branch dies with an error looking for
master.  When there are two or more branches, a sync does
nothing due to branch detection code, but that is expected.

Using sync --branch to try to update just a particular branch
updates no branch, but appears to succeed.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9806-git-p4-options.sh | 53 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index fa40cc8..844aae0 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -27,14 +27,59 @@ test_expect_success 'clone no --git-dir' '
 	test_must_fail git p4 clone --git-dir=xx //depot
 '
 
-test_expect_success 'clone --branch' '
+test_expect_failure 'clone --branch should checkout master' '
 	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot &&
 	test_when_finished cleanup_git &&
 	(
 		cd "$git" &&
-		git ls-files >files &&
-		test_line_count = 0 files &&
-		test_path_is_file .git/refs/remotes/p4/sb
+		git rev-parse refs/remotes/p4/sb >sb &&
+		git rev-parse refs/heads/master >master &&
+		test_cmp sb master &&
+		git rev-parse HEAD >head &&
+		test_cmp sb head
+	)
+'
+
+test_expect_failure 'sync when branch is not called master should work' '
+	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot@2 &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git p4 sync &&
+		git show -s --format=%s refs/remotes/p4/sb >show &&
+		grep "change 3" show
+	)
+'
+
+# engages --detect-branches code, which will do filename filtering so
+# no sync to either b1 or b2
+test_expect_success 'sync when two branches but no master should noop' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git init &&
+		git p4 sync --branch=refs/remotes/p4/b1 //depot@2 &&
+		git p4 sync --branch=refs/remotes/p4/b2 //depot@2 &&
+		git p4 sync &&
+		git show -s --format=%s refs/remotes/p4/b1 >show &&
+		grep "Initial import" show &&
+		git show -s --format=%s refs/remotes/p4/b2 >show &&
+		grep "Initial import" show
+	)
+'
+
+test_expect_failure 'sync --branch updates specified branch' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git init &&
+		git p4 sync --branch=refs/remotes/p4/b1 //depot@2 &&
+		git p4 sync --branch=refs/remotes/p4/b2 //depot@2 &&
+		git p4 sync --branch=refs/remotes/p4/b2 &&
+		git show -s --format=%s refs/remotes/p4/b1 >show &&
+		grep "Initial import" show &&
+		git show -s --format=%s refs/remotes/p4/b2 >show &&
+		grep "change 3" show
 	)
 '
 
-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply related

* [PATCH 00/14] git p4 branch handling fixes
From: Pete Wyckoff @ 2013-01-15  0:46 UTC (permalink / raw)
  To: git; +Cc: Olivier Delalleau

There are multiple oddities in how git-p4 treats multiple
p4 branches, as created with "clone" or "sync" and the
'--branch' argument.  Olivier reported some of these recently
in http://thread.gmane.org/gmane.comp.version-control.git/212613

There are two observable behavior changes, but they
are in the category of "bug fixes" in my opinion:

    - p4/HEAD symbolic ref is always created now; it used to
      be created only after the first sync operation after a clone

    - using clone --branch now checks out files; it used to
      complain that there was no p4/master ref

Pete Wyckoff (14):
  git p4: test sync/clone --branch behavior
  git p4: rearrange and simplify hasOrigin handling
  git p4: add comments to p4BranchesInGit
  git p4: inline listExistingP4GitBranches
  git p4: create p4/HEAD on initial clone
  git p4: verify expected refs in clone --bare test
  git p4: clone --branch should checkout master
  git p4 doc: fix branch detection example
  git p4: allow short ref names to --branch
  git p4: rearrange self.initialParent use
  git p4: fail gracefully on sync with no master branch
  git p4: fix sync --branch when no master branch
  git p4 test: keep P4CLIENT changes inside subshells
  git p4: fix submit when no master branch

 Documentation/git-p4.txt  |  22 +++++--
 git-p4.py                 | 152 ++++++++++++++++++++++++++++++++--------------
 t/t9800-git-p4-basic.sh   |   9 ++-
 t/t9806-git-p4-options.sh | 128 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 253 insertions(+), 58 deletions(-)

-- 
1.8.1.350.gdbf6fd0

^ permalink raw reply

* Re: [PATCH v2] Make git selectively and conditionally ignore certain stat fields
From: Robin Rosenberg @ 2013-01-15  0:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, j sixt
In-Reply-To: <7vy5fv71ad.fsf@alter.siamese.dyndns.org>


> Is this "the user edits in eclipse and then runs 'git status' from
> the
> terminal" problem?

Yes. Of course not just status, but any command that validates
the index. On Unix this is usually bearable, though slow, but on
Windows I often see git status take minutes (yes large files...).

-- robin

^ permalink raw reply

* Re: [PATCH v2 2/3] push: Add support for pre-push hooks
From: Junio C Hamano @ 2013-01-15  0:36 UTC (permalink / raw)
  To: Aaron Schrab; +Cc: git, peff
In-Reply-To: <1358054224-7710-3-git-send-email-aaron@schrab.com>

Aaron Schrab <aaron@schrab.com> writes:

>  t/t5571-pre-push-hook.sh   | 129 +++++++++++++++++++++++++++++++++++++++++++++
> diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
> new file mode 100755
> index 0000000..d68fed7
> --- /dev/null
> +++ b/t/t5571-pre-push-hook.sh
> @@ -0,0 +1,129 @@
> +#!/bin/sh
> +
> +test_description='check pre-push hooks'
> +. ./test-lib.sh
> +
> +# Setup hook that always succeeds
> +HOOKDIR="$(git rev-parse --git-dir)/hooks"
> +HOOK="$HOOKDIR/pre-push"
> +mkdir -p "$HOOKDIR"
> +write_script "$HOOK" <<EOF
> +exit 0
> +EOF

As this script is expected to read from the pipe, if this exits
before the parent has a chance to write to the pipe, the parent can
be killed with sigpipe.

At least the attached patch is necessary.

In the longer term, we may want to discuss what should happen when
the hook exited without even reading what we fed.  My gut feeling is
that we can still trust its exit status (a hook that was badly coded
so it wanted to read from us and use that information to decide but
somehow died before fully reading from us is not likely to exit with
zero status, so we wouldn't diagnosing breakage as a success), but
there may be downsides for being that lax.

If we decide we want to be lax, then the call site of this hook and
the pre-receive hook (is there any other "take info from the
standard input" hook?) need to be modified so that they ignore
sigpipe, I think.

There was a related discussion around this issue about a year ago.

http://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291


 t/t5571-pre-push-hook.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index d68fed7..577d252 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -8,6 +8,7 @@ HOOKDIR="$(git rev-parse --git-dir)/hooks"
 HOOK="$HOOKDIR/pre-push"
 mkdir -p "$HOOKDIR"
 write_script "$HOOK" <<EOF
+cat >/dev/null
 exit 0
 EOF
 
@@ -19,6 +20,7 @@ test_expect_success 'setup' '
 	git push parent1 HEAD:foreign
 '
 write_script "$HOOK" <<EOF
+cat >/dev/null
 exit 1
 EOF
 
@@ -38,6 +40,7 @@ COMMIT2="$(git rev-parse HEAD)"
 export COMMIT2
 
 write_script "$HOOK" <<'EOF'
+cat >/dev/null
 echo "$1" >actual
 echo "$2" >>actual
 cat >>actual

^ permalink raw reply related

* Re: [PATCH v2 0/3] pre-push hook support
From: Junio C Hamano @ 2013-01-15  0:24 UTC (permalink / raw)
  To: Aaron Schrab; +Cc: git
In-Reply-To: <7va9sb8jg7.fsf@alter.siamese.dyndns.org>

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Aaron Schrab <aaron@schrab.com> writes:
>>
>>> Main changes since the initial version:
>>>
>>>  * The first patch converts the existing hook callers to use the new
>>>    find_hook() function.
>>>  * Information about what is to be pushed is now sent over a pipe rather
>>>    than passed as command-line parameters.
>>>
>>> Aaron Schrab (3):
>>>   hooks: Add function to check if a hook exists
>>>   push: Add support for pre-push hooks
>>>   Add sample pre-push hook script
>>
>> Getting much nicer.  Thanks.
>
> Hmph, t5571 seems to be flaky in that it sometimes fails but passes
> when run again.  Something timing dependent is going on???

With this patch applied, repeatedly try to

 - make sure "foreign" ref does not exist; and
 - attempt pushing the HEAD:foreign to create the "foreign" ref

until it fails, I can get it stop before the output scrolls off of
my 114 line terminal.  Then when I revert the changes to transport.[ch]
and builtin/push.c in this series, the test will keep going.

Wait.  The sample hook used in the test _is_ fed some input but it
exits without reading any.  What happens when we fork it, and it
completes execution before we even have a chance to feed a single
byte?  Wont' we get a sigpipe and die?

Yup, I think that is what is missing from run_pre_push_hook()
implementation.

diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index d68fed7..050318b 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -16,8 +16,15 @@ test_expect_success 'setup' '
 	git init --bare repo1 &&
 	git remote add parent1 repo1 &&
 	test_commit one &&
-	git push parent1 HEAD:foreign
+	while :
+	do
+		git push parent1 :refs/heads/foreign &&
+		git push parent1 HEAD:foreign || break
+	done
 '
+
+exit
+
 write_script "$HOOK" <<EOF
 exit 1
 EOF

^ permalink raw reply related

* Re: [PATCH v2] Make git selectively and conditionally ignore certain stat fields
From: Junio C Hamano @ 2013-01-15  0:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git, j sixt
In-Reply-To: <1815551092.2039693.1358207014937.JavaMail.root@dewire.com>

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

> Semantically they're somewhat different. My flags are for ignoring
> a value when it's not used as indicated by the value zero, while
> trustctime is for ignoring untrustworthy, non-zero, values.

Yeah, I realized that after writing that message.

> Another thing that I noticed, is that I probably wanto to be able to filter on the precision
> of timestamps. Again, this i JGit-related. Current JGit has milliseconds precision (max), whereas
> Git has down to nanosecond precision in timestamps. Newer JGits may get nanoseconds timestamps too,
> but on current Linux versions JGit gets only integral seconds regardless of file system. 
>
> Would the names, milli, micro, nano be good for ignoring the tail when zero, or n1..n9 (obviously
> n2 would be ok too). nN = ignore all but first N nsec digits if they are zero)?

It somehow starts to sound like over-engineering to solve a wrong
problem.

I'd say a simplistic "ignore if zero is stored" or even "ignore this
as one of the systems that shares this file writes crap in it" may
be sufficient, and if this is a jGit specific issue, it might even
make sense to introduce a single configuration variable with string
"jgit" somewhere in its name and bypass the stat field comparison
for known-problematic fields, instead of having the user know and
list what stat fields need special attention.

Is this "the user edits in eclipse and then runs 'git status' from the
terminal" problem?

^ permalink raw reply

* Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
From: Johannes Schindelin @ 2013-01-14 23:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Carlos Martín Nieto
In-Reply-To: <7v38y38hhm.fsf@alter.siamese.dyndns.org>

Hi Junio,

On Mon, 14 Jan 2013, Junio C Hamano wrote:

> It appears that memcmp() uses the usual "one word at a time"
> comparison and triggers valgrind in a callback of bsearch() used in
> the refname search.  I can easily trigger problems in any script
> with test_commit (e.g. "sh t0101-at-syntax.sh --valgrind -i -v")
> without this suppression.

I have no way to replicate that issue, but I take your word for it. With
that in mind, here is my ACK.

Ciao,
Johannes

^ permalink raw reply

* [PATCH v3] Make git selectively and conditionally ignore certain stat fields
From: Robin Rosenberg @ 2013-01-14 23:51 UTC (permalink / raw)
  To: git; +Cc: gitster, j.sixt, Robin Rosenberg
In-Reply-To: <7vmwwb8m25.fsf@alter.siamese.dyndns.org>

Specifically the fields uid, gid, ctime, ino and dev are set to zero
by JGit. Any stat checking by git will then need to check content,
which may be very slow, particularly on Windows. Since mtime and size
is typically enough we should allow the user to tell git to avoid
checking these fields if they are set to zero in the index.

This change introduces a core.ignorezerostat config option where the
user can list the fields to ignore using the names above.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 Documentation/config.txt |  9 +++++++++
 cache.h                  |  8 ++++++++
 config.c                 | 26 ++++++++++++++++++++++++++
 environment.c            |  1 +
 read-cache.c             | 29 ++++++++++++++++++-----------
 5 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5809e0..7f34c94 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -235,6 +235,15 @@ core.trustctime::
 	crawlers and some backup systems).
 	See linkgit:git-update-index[1]. True by default.
 
+core.ignorezerostat::
+	Affects the interpretation of some fields in the index. If
+	unset has no effect. When set to a comma separated list of fields,
+	each of the fields in the index will be excluded from comparison with
+	working tree if the index value is zero. The following fields
+	are recognzed: `uid', `gid', `ctime', `ino' and `dev'. When ctime is ignored
+	the setting of 'core.trustctime' is overridden by by this config
+	value.
+
 core.quotepath::
 	The commands that output paths (e.g. 'ls-files',
 	'diff'), when not given the `-z` option, will quote
diff --git a/cache.h b/cache.h
index c257953..524e49a 100644
--- a/cache.h
+++ b/cache.h
@@ -536,6 +536,14 @@ extern int delete_ref(const char *, const unsigned char *sha1, int delopt);
 /* Environment bits from configuration mechanism */
 extern int trust_executable_bit;
 extern int trust_ctime;
+extern int check_nonzero_stat;
+#define CHECK_NONZERO_STAT_UID (1<<0)
+#define CHECK_NONZERO_STAT_GID (1<<1)
+#define CHECK_NONZERO_STAT_CTIME (1<<2)
+#define CHECK_NONZERO_STAT_INO (1<<3)
+#define CHECK_NONZERO_STAT_DEV (1<<4)
+#define CHECK_NONZERO_STAT_MASK ((1<<5)-1)
+
 extern int quote_path_fully;
 extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
diff --git a/config.c b/config.c
index 7b444b6..6b617bc 100644
--- a/config.c
+++ b/config.c
@@ -566,6 +566,32 @@ static int git_default_core_config(const char *var, const char *value)
 		trust_ctime = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "core.ignorezerostat")) {
+		const char *copy;
+		const char *tok;
+		git_config_string(&copy, "core.ignorezerostat", value);
+		check_nonzero_stat = CHECK_NONZERO_STAT_MASK;
+		tok = strtok((char*)copy, ",");
+		while (tok) {
+			if (strcasecmp(tok, "uid") == 0)
+				check_nonzero_stat &= ~CHECK_NONZERO_STAT_UID;
+			else if (strcasecmp(tok, "gid") == 0)
+				check_nonzero_stat &= ~CHECK_NONZERO_STAT_GID;
+			else if (strcasecmp(tok, "ctime") == 0) {
+				check_nonzero_stat &= ~CHECK_NONZERO_STAT_CTIME;
+				trust_ctime = 0;
+			} else if (strcasecmp(tok, "ino") == 0)
+				check_nonzero_stat &= ~CHECK_NONZERO_STAT_INO;
+			else if (strcasecmp(tok, "dev") == 0)
+				check_nonzero_stat &= ~CHECK_NONZERO_STAT_DEV;
+			else
+				die_bad_config(var);
+			tok = strtok(NULL, ",");
+		}
+		if (check_nonzero_stat >= CHECK_NONZERO_STAT_MASK)
+			die_bad_config(var);
+		free((char*)copy);
+	}
 
 	if (!strcmp(var, "core.quotepath")) {
 		quote_path_fully = git_config_bool(var, value);
diff --git a/environment.c b/environment.c
index 85edd7f..e90b52f 100644
--- a/environment.c
+++ b/environment.c
@@ -13,6 +13,7 @@
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
+int check_nonzero_stat = CHECK_NONZERO_STAT_MASK;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = 7;
 int ignore_case;
diff --git a/read-cache.c b/read-cache.c
index fda78bc..c4226ee 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -197,21 +197,27 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 	}
 	if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
 		changed |= MTIME_CHANGED;
-	if (trust_ctime && ce->ce_ctime.sec != (unsigned int)st->st_ctime)
-		changed |= CTIME_CHANGED;
+	if ((trust_ctime || ((check_nonzero_stat & CHECK_NONZERO_STAT_CTIME) && ce->ce_ctime.sec)))
+		if (ce->ce_ctime.sec != (unsigned int)st->st_ctime)
+			changed |= CTIME_CHANGED;
 
 #ifdef USE_NSEC
 	if (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))
 		changed |= MTIME_CHANGED;
-	if (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
-		changed |= CTIME_CHANGED;
+	if ((trust_ctime || ((check_nonzero_stat & CHECK_NONZERO_STAT_CTIME) && ce->ce_ctime.nsec))
+		if (ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
+			changed |= CTIME_CHANGED;
 #endif
 
-	if (ce->ce_uid != (unsigned int) st->st_uid ||
-	    ce->ce_gid != (unsigned int) st->st_gid)
-		changed |= OWNER_CHANGED;
-	if (ce->ce_ino != (unsigned int) st->st_ino)
-		changed |= INODE_CHANGED;
+	if ((check_nonzero_stat & CHECK_NONZERO_STAT_UID) || ce->ce_uid)
+		if (ce->ce_uid != (unsigned int) st->st_uid)
+			changed |= OWNER_CHANGED;
+	if ((check_nonzero_stat & CHECK_NONZERO_STAT_GID) || ce->ce_gid)
+		if (ce->ce_gid != (unsigned int) st->st_gid)
+			changed |= OWNER_CHANGED;
+	if ((check_nonzero_stat & CHECK_NONZERO_STAT_INO) || ce->ce_ino)
+		if (ce->ce_ino != (unsigned int) st->st_ino)
+			changed |= INODE_CHANGED;
 
 #ifdef USE_STDEV
 	/*
@@ -219,8 +225,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 	 * clients will have different views of what "device"
 	 * the filesystem is on
 	 */
-	if (ce->ce_dev != (unsigned int) st->st_dev)
-		changed |= INODE_CHANGED;
+	if ((check_nonzero_stat & CHECK_NONZERO_STAT_DEV) || ce->ce_dev)
+		if (ce->ce_dev != (unsigned int) st->st_dev)
+			changed |= INODE_CHANGED;
 #endif
 
 	if (ce->ce_size != (unsigned int) st->st_size)
-- 
1.8.1.337.gc903ef9.dirty

^ permalink raw reply related

* Re: Error:non-monotonic index after failed recursive "sed" command
From: Philip Oakley @ 2013-01-14 23:45 UTC (permalink / raw)
  To: George Karpenkov, Matthieu Moy; +Cc: Junio C Hamano, Johannes Sixt, git
In-Reply-To: <CAMoGvRKMwP_JBvNNWoN=m9AX3MP9xVgBUwxELHtry_-8Um8WKQ@mail.gmail.com>

From: "George Karpenkov" <george@metaworld.ru>
Sent: Monday, January 14, 2013 10:57 PM
> Thanks everyone!
>
> Progress so far:
>
> After executing reverse sed command:
> find .git -name '*.*' -exec sed -i 's/    /\t/g' {} \;

Have you counted how many substitutions there are in the pack file(s). 
It may be sufficiently small that you can  simply try all the possible 
combinations of fwd and reverse substitutions. Even if it takes a few 
days the computer won't get bored ;-)

>
> And trying to switch the branch I get:
>
>> git checkout X
>
> error: failed to read object 51a980792f26875d00acb79a19f043420f542cfa
> at offset 41433013 from
> .git/objects/pack/pack-8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack
> fatal: packed object 51a980792f26875d00acb79a19f043420f542cfa (stored
> in .git/objects/pack/pack-
> 8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack) is corrupt
>
> So the actual .pack file is corrupt, unfortunately.
>
> On Tue, Jan 15, 2013 at 6:13 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Everybody seems to be getting an impression that .idx is the only
>>> thing that got corrupt.  Where does that come from?
>>
>> It's the only thing that appear in the error message. This does not
>> imply that it is the only corrupt thing, but gives a little hope that 
>> it
>> may still be the case.
>>
>> Actually, I thought the "read-only" protection should have protected
>> files in object/ directory, but a little testing shows that "sed -i"
>> gladly accepts to modify read-only files (technically, it does not
>> modify it, but creates a temporary file with the new content, and 
>> then
>> renames it to the new location). So, the hope that pack files are
>> uncorrupted is rather thin unfortunately.
>>
>> --
>> Matthieu Moy
>> http://www-verimag.imag.fr/~moy/
> --

^ permalink raw reply

* Re: [PATCH v2] Make git selectively and conditionally ignore certain stat fields
From: Robin Rosenberg @ 2013-01-14 23:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, j sixt
In-Reply-To: <7vmwwb8m25.fsf@alter.siamese.dyndns.org>



----- Ursprungligt meddelande -----
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> 
> > diff --git a/read-cache.c b/read-cache.c
> > index fda78bc..f7fe15d 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -197,8 +197,9 @@ static int ce_match_stat_basic(struct
> > cache_entry *ce, struct stat *st)
> >  	}
> >  	if (ce->ce_mtime.sec != (unsigned int)st->st_mtime)
> >  		changed |= MTIME_CHANGED;
> > -	if (trust_ctime && ce->ce_ctime.sec != (unsigned
> > int)st->st_ctime)
> > -		changed |= CTIME_CHANGED;
> > +	if ((trust_ctime ||
> > ((check_nonzero_stat&CHECK_NONZERO_STAT_CTIME) &&
> > ce->ce_ctime.sec)))
> 
> One SP is required on each side of a binary operator; please have
> one after check_nonzero_stat and after the & after it.
> 
> I wonder if we should lose the trust_ctime variable and use this
> check_nonzero_stat bitset exclusively, provided that this were a
> good direction to go?

Semantically they're somewhat different. My flags are for ignoring
a value when it's not used as indicated by the value zero, while
trustctime is for ignoring untrustworthy, non-zero, values.

>From 1ce4790bf5e:
    A new configuration variable 'core.trustctime' is introduced to
    allow ignoring st_ctime information when checking if paths
    in the working tree has changed, because there are situations where
    it produces too much false positives.  Like when file system crawlers
    keep changing it when scanning and using the ctime for marking scanned
    files.

(your second mail)
>Also I am getting these:
>
>config.c: In function 'git_default_core_config':
>config.c:571: error: passing argument 1 of 'git_config_string' from incompatible pointer type
>config.c:540: note: expected 'const char **' but argument is of type 'char **'
>config.c:573: error: passing argument 1 of 'strtok' discards qualifiers from pointer target type

Different compilers have different defaults. I'm on OS X (mountain lion), or am I missing
something? I do get a warning. Am I allowed to modify the value, like strtok does? Seems I
missed the opportunity to use the copy rather then the original value.

Another thing that I noticed, is that I probably wanto to be able to filter on the precision
of timestamps. Again, this i JGit-related. Current JGit has milliseconds precision (max), whereas
Git has down to nanosecond precision in timestamps. Newer JGits may get nanoseconds timestamps too,
but on current Linux versions JGit gets only integral seconds regardless of file system. 

Would the names, milli, micro, nano be good for ignoring the tail when zero, or n1..n9 (obviously
n2 would be ok too). nN = ignore all but first N nsec digits if they are zero)?

-- robin

^ permalink raw reply

* [RFC/PATCH] ignore memcmp() overreading in bsearch() callback
From: Junio C Hamano @ 2013-01-14 23:36 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Carlos Martín Nieto, Johannes Schindelin

It appears that memcmp() uses the usual "one word at a time"
comparison and triggers valgrind in a callback of bsearch() used in
the refname search.  I can easily trigger problems in any script
with test_commit (e.g. "sh t0101-at-syntax.sh --valgrind -i -v")
without this suppression.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/valgrind/default.supp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 0a6724f..032332f 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -49,3 +49,11 @@
 	Memcheck:Addr4
 	fun:copy_ref
 }
+
+{
+	ignore-memcmp-reading-too-much-in-bsearch-callback
+	Memcheck:Addr4
+	fun:ref_entry_cmp_sslice
+	fun:bsearch
+	fun:search_ref_dir
+}

^ permalink raw reply related

* Re: Error:non-monotonic index after failed recursive "sed" command
From: George Karpenkov @ 2013-01-14 22:57 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Junio C Hamano, Johannes Sixt, git
In-Reply-To: <vpqobgrpoh7.fsf@grenoble-inp.fr>

Thanks everyone!

Progress so far:

After executing reverse sed command:
find .git -name '*.*' -exec sed -i 's/    /\t/g' {} \;

And trying to switch the branch I get:

> git checkout X

error: failed to read object 51a980792f26875d00acb79a19f043420f542cfa
at offset 41433013 from
.git/objects/pack/pack-8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack
fatal: packed object 51a980792f26875d00acb79a19f043420f542cfa (stored
in .git/objects/pack/pack-
8d629235ee9fec9c6683d42e3edb21a1b0f6e027.pack) is corrupt

So the actual .pack file is corrupt, unfortunately.

On Tue, Jan 15, 2013 at 6:13 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Everybody seems to be getting an impression that .idx is the only
>> thing that got corrupt.  Where does that come from?
>
> It's the only thing that appear in the error message. This does not
> imply that it is the only corrupt thing, but gives a little hope that it
> may still be the case.
>
> Actually, I thought the "read-only" protection should have protected
> files in object/ directory, but a little testing shows that "sed -i"
> gladly accepts to modify read-only files (technically, it does not
> modify it, but creates a temporary file with the new content, and then
> renames it to the new location). So, the hope that pack files are
> uncorrupted is rather thin unfortunately.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH v2 0/3] pre-push hook support
From: Junio C Hamano @ 2013-01-14 22:54 UTC (permalink / raw)
  To: Aaron Schrab; +Cc: git
In-Reply-To: <7vk3rfek51.fsf@alter.siamese.dyndns.org>

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

> Aaron Schrab <aaron@schrab.com> writes:
>
>> Main changes since the initial version:
>>
>>  * The first patch converts the existing hook callers to use the new
>>    find_hook() function.
>>  * Information about what is to be pushed is now sent over a pipe rather
>>    than passed as command-line parameters.
>>
>> Aaron Schrab (3):
>>   hooks: Add function to check if a hook exists
>>   push: Add support for pre-push hooks
>>   Add sample pre-push hook script
>
> Getting much nicer.  Thanks.

Hmph, t5571 seems to be flaky in that it sometimes fails but passes
when run again.  Something timing dependent is going on???

^ permalink raw reply

* git grep performance regression
From: Ross Lagerwall @ 2013-01-14 22:38 UTC (permalink / raw)
  To: git; +Cc: Jean-Noël AVILA

Hi,

I have noticed a performance regression in git grep between v1.8.1 and
v1.8.1.1:

On the kernel tree:
For git 1.8.1:
$ time git grep foodsgsg

real   0m0.158s
user   0m0.290s
sys    0m0.207s

For git 1.8.1.1:
$ time /tmp/g/bin/git grep foodsgsg

real   0m0.501s
user   0m0.707s
sys    0m0.493s


A bisect seems to indicate that it was introduced by 94bc67:
commit 94bc671a1f2e8610de475c2494d2763355a99f65
Author: Jean-Noël AVILA <avila.jn@gmail.com>
Date:   Sat Dec 8 21:04:39 2012 +0100

    Add directory pattern matching to attributes
    
    The manpage of gitattributes says: "The rules how the pattern
    matches paths are the same as in .gitignore files" and the gitignore
    pattern matching has a pattern ending with / for directory matching.
    
    This rule is specifically relevant for the 'export-ignore' rule used
    for git archive.
    
    Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Regards
-- 
Ross Lagerwall

^ permalink raw reply

* [PATCH v2] am: invoke perl's strftime in C locale
From: Dmitry V. Levin @ 2013-01-14 22:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4ln8mgd.fsf@alter.siamese.dyndns.org>

This fixes "hg" patch format support for locales other than C and en_*.
Before the change, git-am was making "Date:" line from hg changeset
metadata according to the current locale, and this line was rejected
later with "invalid date format" diagnostics because localized date
strings are not supported.

Reported-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---

 v2: replaced "unfriendly" URL with a short description

 git-am.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index c682d34..64b88e4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -334,7 +334,7 @@ split_patches () {
 			# Since we cannot guarantee that the commit message is in
 			# git-friendly format, we put no Subject: line and just consume
 			# all of the message as the body
-			perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+			LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 				if ($subject) { print ; }
 				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
 				elsif (/^\# Date /) {

-- 
ldv

^ permalink raw reply related


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