Git development
 help / color / mirror / Atom feed
* Re: A Python script to put CTAN into git (from DVDs)
From: Jonathan Fine @ 2011-11-07 22:03 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: python-list, git
In-Reply-To: <m3ipmv38o2.fsf@localhost.localdomain>

On 07/11/11 21:49, Jakub Narebski wrote:

[snip]

> But now I understand that you are just building tree objects, and
> creating references to them (with implicit ordering given by names,
> I guess).  This is to be a start of further work, isn't it?

Yes, that's exactly the point, and my apologies if I was not clear enough.

I'll post again when I've finished the script and performed placed 
several years of DVD into git.  Then the discussion will be more 
concrete - we have this tree, how do we make it more useful.

Thank you for your contributions, particularly telling me about gitpan.

-- 
Jonathan

^ permalink raw reply

* Re: A Python script to put CTAN into git (from DVDs)
From: Jakub Narebski @ 2011-11-07 21:50 UTC (permalink / raw)
  To: Jonathan Fine; +Cc: python-list, git
In-Reply-To: <4EB83DDD.1080103@pytex.org>

The following message is a courtesy copy of an article
that has been posted to comp.text.tex as well.

Jonathan Fine <jfine@pytex.org> writes:
> On 06/11/11 20:28, Jakub Narebski wrote:
> 
> > Note that for gitPAN each "distribution" (usually but not always
> > corresponding to single Perl module) is in separate repository.
> > The dependencies are handled by CPAN / CPANPLUS / cpanm client
> > (i.e. during install).
> 
> Thank you for your interest, Jakub, and also for this information.
> With TeX there's a difficult which Perl, I think, does not have.  With
> TeX we process documents, which may demand specific versions of
> packages. LaTeX users are concerned that move on to a later version
> will cause documents to break.

How you can demand specific version of package?

In the "\usepackage[options]{packages}[version]" LaTeX command the
<version> argument specifies _minimal_ (oldest) version.  The same
is true for Perl "use Module VERSION LIST".
 
Nevertheless while with "use Module VERSION" / "use Module VERSION LIST"
you can request minimal version of Perl Module, the META build-time spec 
can include requirement of exact version of required package:

http://p3rl.org/CPAN::Meta::Spec

  Version Ranges
  ~~~~~~~~~~~~~~

  Some fields (prereq, optional_features) indicate the particular
  version(s) of some other module that may be required as a
  prerequisite. This section details the Version Range type used to
  provide this information.

  The simplest format for a Version Range is just the version number
  itself, e.g. 2.4. This means that *at least* version 2.4 must be
  present. To indicate that *any* version of a prerequisite is okay,
  even if the prerequisite doesn't define a version at all, use the
  version 0.

  Alternatively, a version range *may* use the operators < (less than),
  <= (less than or equal), > (greater than), >= (greater than or
  equal), == (equal), and != (not equal). For example, the
  specification < 2.0 means that any version of the prerequisite less
  than 2.0 is suitable.

  For more complicated situations, version specifications *may* be
  AND-ed together using commas. The specification >= 1.2, != 1.5, <
  2.0 indicates a version that must be *at least* 1.2, *less than* 2.0,
  and *not equal to* 1.5.

> > Putting all DVD (is it "TeX Live" DVD by the way?) into single
> > repository would put quite a bit of stress to git; it was created for
> > software development (although admittedly of large project like Linux
> > kernel), not 4GB+ trees.
> 
> I'm impressed by how well git manages it.  It took about 15 minutes to
> build the 4GB tree, and it was disk speed rather than CPU which was
> the bottleneck.

I still think that using modified contrib/fast-import/import-zips.py
(or import-tars.perl, or import-directories.perl) would be a better
solution here...
 
[...]
> We may be at cross purposes.  My first task is get the DVD tree into
> git, performing necessary transformations such as expanding zip files
> along the way.  Breaking the content into submodules can, I believe,
> be done afterwards.

'reposurgeon' might help there... or might not.  Same with git-subtree
tool.

But now I understand that you are just building tree objects, and
creating references to them (with implicit ordering given by names,
I guess).  This is to be a start of further work, isn't it?

> With DVDs from several years it could take several hours to load
> everything into git.  For myself, I'd like to do that once, more or
> less as a batch process, and then move on to the more interesting
> topics. Getting the DVD contents into git is already a significant
> piece of work.
> 
> Once done, I can them move on to what you're interested in, which is
> organising the material.  And I hope that others in the TeX community
> will get involved with that, because I'm not building this repository
> just for myself.

[...]

> > > In addition, many TeX users have a TeX DVD.  If they import it into a
> > > git repository (using for example my script) then the update from 2011
> > > to 2012 would require much less bandwidth.
> >
> > ???
> 
> A quick way to bring your TeX distribution up to date is to do a delta
> with a later distribution, and download the difference.  That's what
> git does, and it does it well.  So I'm keen to convert a TeX DVD into
> a git repository, and then differences can be downloaded.

Here perhaps you should take a look at git-based 'bup' backup system.

Anyway I am not sure if for git to be able to generate deltas well you
have to have DAG of commits, so Git can notice what you have and what
you have not.  Trees might be not enough here. (!)
 
> > Commit = tree + parent + metadata.
> 
> Actually, any number of parents, including none.  What metadata do I
> have to provide?  At this time nothing, I think, beyond that provided
> by the name of a reference (to the root of a tree).

Metadata = commit message (here you can e.g. put the official name of
DVD), author and committer info (name, email, date and time, timezone;
date and time you can get from mtime / creation time of DVD). 

[cut]
 
-- 
Jakub Narębski

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Junio C Hamano @ 2011-11-07 21:48 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git,
	Jim Meyering, Fredrik Gustafsson, Andreas Schwab
In-Reply-To: <20111107213219.GA13537@elie.hsd1.il.comcast.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> So, the purpose of this patch was to work around this common bug in
> static analyzers.

I fail to see how it could be even considered a work around.

If you do not use static analyzers, you do not have to do such a change,
and the resulting code would (not the "negative" side, but the "positive"
side) catch real bugs when somebody screwes up and stuffs a bogus oob
value in p->field.

With the removal of the check, you _have_ to rely on static analyzers to
do the _right thing_, but if you have static analyzers that do the right
thing, you do not have to have such a workaround to begin with.

^ permalink raw reply

* Re: pretty placeholders for reflog entries
From: Jack Nagel @ 2011-11-07 21:41 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111107211325.GB7380@sigill.intra.peff.net>

On Mon, Nov 7, 2011 at 3:13 PM, Jeff King <peff@peff.net> wrote:
> I doubt it would be a very big patch. Want to get your feet wet in git
> development? :)

I'll certainly give it a shot if I find some free time to do so. Though if
someone else felt like doing it in the meantime, my feelings wouldn't be
hurt. =)

Jack

^ permalink raw reply

* [PATCH 4/4] git-p4: add test for p4 labels
From: Luke Diamand @ 2011-11-07 21:36 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1320701799-26071-1-git-send-email-luke@diamand.org>

Add basic test of p4 label import. Checks label import and
import with shell metachars; labels with different length
descriptions.
---
 t/t9804-git-p4-label.sh |   73 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)
 create mode 100755 t/t9804-git-p4-label.sh

diff --git a/t/t9804-git-p4-label.sh b/t/t9804-git-p4-label.sh
new file mode 100755
index 0000000..eba3521
--- /dev/null
+++ b/t/t9804-git-p4-label.sh
@@ -0,0 +1,73 @@
+test_description='git-p4 p4 label tests'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+# Basic p4 label tests.
+#
+# Note: can't have more than one label per commit - others
+# are silently discarded.
+#
+test_expect_success 'basic p4 labels' '
+	test_when_finished cleanup_git &&
+	(
+		cd "$cli" &&
+		mkdir -p main &&
+
+		echo f1 >main/f1 &&
+		p4 add main/f1 &&
+		p4 submit -d "main/f1" &&
+
+		echo f2 >main/f2 &&
+		p4 add main/f2 &&
+		p4 submit -d "main/f2" &&
+
+		echo f3 >main/file_with_\$metachar &&
+		p4 add main/file_with_\$metachar &&
+		p4 submit -d "file with metachar" &&
+
+		p4 tag -l tag_f1_only main/f1 &&
+		p4 tag -l tag_with\$_shell_char main/... &&
+
+		echo f4 >main/f4 &&
+		p4 add main/f4 &&
+		p4 submit -d "main/f4" &&
+
+		p4 label -i <<-EOF &&
+		Label: long_label
+		Description:
+		   A Label first line
+		   A Label second line
+		View:	//depot/...
+		EOF
+
+		p4 tag -l long_label ... &&
+
+		p4 labels ... &&
+
+		cd "$git" &&
+		pwd &&
+		"$GITP4" clone --dest=. --detect-labels //depot@all &&
+
+		git tag &&
+		git tag >taglist &&
+		test_line_count = 3 taglist &&
+
+		cd main &&
+		git checkout tag_tag_f1_only &&
+		! test -f f2 &&
+		git checkout tag_tag_with\$_shell_char &&
+		test -f f1 && test -f f2 && test -f file_with_\$metachar &&
+
+		git show tag_long_label | grep -q "A Label second line"
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.7.295.g34dd4

^ permalink raw reply related

* [PATCH 3/4] git-p4: importing labels should cope with missing owner
From: Luke Diamand @ 2011-11-07 21:36 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1320701799-26071-1-git-send-email-luke@diamand.org>

In p4, the Owner field is optional. If it is missing,
construct something sensible rather than crashing.
---
 contrib/fast-import/git-p4 |   45 +++++++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 02f0f54..d97f927 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -553,6 +553,27 @@ class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
         self.needsGit = True
+        self.myP4UserId = None
+
+    def p4UserId(self):
+        if self.myP4UserId:
+            return self.myP4UserId
+
+        results = p4CmdList("user -o")
+        for r in results:
+            if r.has_key('User'):
+                self.myP4UserId = r['User']
+                return r['User']
+        die("Could not find your p4 user id")
+
+    def p4UserIsMe(self, p4User):
+        # return True if the given p4 user is actually me
+        me = self.p4UserId()
+        if not p4User or p4User != me:
+            return False
+        else:
+            return True
+
 
 class P4UserMap:
     def __init__(self):
@@ -694,7 +715,6 @@ class P4Submit(Command, P4UserMap):
         self.verbose = False
         self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
         self.isWindows = (platform.system() == "Windows")
-        self.myP4UserId = None
 
     def check(self):
         if len(p4CmdList("opened ...")) > 0:
@@ -802,25 +822,6 @@ class P4Submit(Command, P4UserMap):
                     return 1
         return 0
 
-    def p4UserId(self):
-        if self.myP4UserId:
-            return self.myP4UserId
-
-        results = p4CmdList("user -o")
-        for r in results:
-            if r.has_key('User'):
-                self.myP4UserId = r['User']
-                return r['User']
-        die("Could not find your p4 user id")
-
-    def p4UserIsMe(self, p4User):
-        # return True if the given p4 user is actually me
-        me = self.p4UserId()
-        if not p4User or p4User != me:
-            return False
-        else:
-            return True
-
     def prepareSubmitTemplate(self):
         # remove lines in the Files section that show changes to files outside the depot path we're committing into
         template = ""
@@ -1506,7 +1507,9 @@ class P4Sync(Command, P4UserMap):
 
                     owner = labelDetails["Owner"]
                     tagger = ""
-                    if author in self.users:
+                    if not owner:
+                        tagger = "%s <a@b> %s %s" % (self.p4UserId(), epoch, self.tz)
+                    elif author in self.users:
                         tagger = "%s %s %s" % (self.users[owner], epoch, self.tz)
                     else:
                         tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
-- 
1.7.7.295.g34dd4

^ permalink raw reply related

* [PATCH 2/4] git-p4: cope with labels with empty descriptions
From: Luke Diamand @ 2011-11-07 21:36 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1320701799-26071-1-git-send-email-luke@diamand.org>

Use an explicit length for the data in a label, rather
than EOT, so that labels with empty descriptions are
passed through correctly.
---
 contrib/fast-import/git-p4 |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index bcac6ec..02f0f54 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1511,9 +1511,11 @@ class P4Sync(Command, P4UserMap):
                     else:
                         tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)
                     self.gitStream.write("tagger %s\n" % tagger)
-                    self.gitStream.write("data <<EOT\n")
-                    self.gitStream.write(labelDetails["Description"])
-                    self.gitStream.write("EOT\n\n")
+
+                    description = labelDetails["Description"]
+                    self.gitStream.write("data %d\n" % len(description))
+                    self.gitStream.write(description)
+                    self.gitStream.write("\n")
 
                 else:
                     if not self.silent:
-- 
1.7.7.295.g34dd4

^ permalink raw reply related

* [PATCH 1/4] git-p4: handle p4 branches and labels containing shell chars
From: Luke Diamand @ 2011-11-07 21:36 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Luke Diamand
In-Reply-To: <1320701799-26071-1-git-send-email-luke@diamand.org>

Don't use shell expansion when detecting branches, as it will
fail if the branch name contains a shell metachar. Similarly
for labels.

Add additional test for branches with shell metachars.
---
 contrib/fast-import/git-p4 |    8 +++---
 t/t9801-git-p4-branch.sh   |   48 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index b975d67..bcac6ec 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -793,7 +793,7 @@ class P4Submit(Command, P4UserMap):
     def canChangeChangelists(self):
         # check to see if we have p4 admin or super-user permissions, either of
         # which are required to modify changelists.
-        results = p4CmdList("protects %s" % self.depotPath)
+        results = p4CmdList(["protects", self.depotPath])
         for r in results:
             if r.has_key('perm'):
                 if r['perm'] == 'admin':
@@ -1528,7 +1528,7 @@ class P4Sync(Command, P4UserMap):
     def getLabels(self):
         self.labels = {}
 
-        l = p4CmdList("labels %s..." % ' '.join (self.depotPaths))
+        l = p4CmdList(["labels", "%s..." % ' '.join (self.depotPaths)])
         if len(l) > 0 and not self.silent:
             print "Finding files belonging to labels in %s" % `self.depotPaths`
 
@@ -1570,7 +1570,7 @@ class P4Sync(Command, P4UserMap):
             command = "branches"
 
         for info in p4CmdList(command):
-            details = p4Cmd("branch -o %s" % info["branch"])
+            details = p4Cmd(["branch", "-o", info["branch"]])
             viewIdx = 0
             while details.has_key("View%s" % viewIdx):
                 paths = details["View%s" % viewIdx].split(" ")
@@ -1708,7 +1708,7 @@ class P4Sync(Command, P4UserMap):
         sourceRef = self.gitRefForBranch(sourceBranch)
         #print "source " + sourceBranch
 
-        branchParentChange = int(p4Cmd("changes -m 1 %s...@1,%s" % (sourceDepotPath, firstChange))["change"])
+        branchParentChange = int(p4Cmd(["changes", "-m", "1", "%s...@1,%s" % (sourceDepotPath, firstChange)])["change"])
         #print "branch parent: %s" % branchParentChange
         gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange)
         if len(gitParent) > 0:
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index a25f18d..bd08dff 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -226,6 +226,54 @@ test_expect_success 'git-p4 clone simple branches' '
 	)
 '
 
+# Create a branch with a shell metachar in its name
+#
+# 1. //depot/main
+# 2. //depot/branch$3
+
+test_expect_success 'branch with shell char' '
+	test_when_finished cleanup_git &&
+	test_create_repo "$git" &&
+	(
+		cd "$cli" &&
+
+		mkdir -p main &&
+
+		echo f1 >main/f1 &&
+		p4 add main/f1 &&
+		p4 submit -d "main/f1" &&
+
+		p4 integrate //depot/main/... //depot/branch\$3/... &&
+		p4 submit -d "integrate main to branch\$3" &&
+
+		echo f1 >branch\$3/shell_char_branch_file &&
+		p4 add branch\$3/shell_char_branch_file &&
+		p4 submit -d "branch\$3/shell_char_branch_file" &&
+
+		p4 branch -i <<-EOF &&
+		Branch: branch\$3
+		View: //depot/main/... //depot/branch\$3/...
+		EOF
+
+		p4 edit main/f1 &&
+		echo "a change" >> main/f1 &&
+		p4 submit -d "a change" main/f1 &&
+
+		p4 integrate -b branch\$3 &&
+		p4 resolve -am branch\$3/... &&
+		p4 submit -d "integrate main to branch\$3" &&
+
+		cd "$git" &&
+
+		git config git-p4.branchList main:branch\$3 &&
+		"$GITP4" clone --dest=. --detect-branches //depot@all &&
+		git log --all --graph --decorate --stat &&
+		git reset --hard p4/depot/branch\$3 &&
+		test -f shell_char_branch_file &&
+		test -f f1
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.7.295.g34dd4

^ permalink raw reply related

* [RFC/PATCH 0/4] git-p4: small fixes to branches and labels; tests
From: Luke Diamand @ 2011-11-07 21:36 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Luke Diamand

This is a small set of patches to git-p4 to fix a couple of issues with
branches and labels.

Firstly, I've added the fixes needed so that branches and labels can
contain shell metacharacters (missed from the previous series). Added
a test case for this.

In adding the test case for labels I also found a few other small bugs
in the label handling:

 - labels missing a description or "EOT" in their text cause problems;
 - labels without an owner cause problems.

I also noticed, but did not fix, that you can't have more than one label
per commit (the others are silently dropped) and the documentation for
branch import could be improved.

Luke Diamand (4):
  git-p4: handle p4 branches and labels containing shell chars
  git-p4: cope with labels with empty descriptions
  git-p4: importing labels should cope with missing owner
  git-p4: add test for p4 labels

 contrib/fast-import/git-p4 |   61 ++++++++++++++++++++-----------------
 t/t9801-git-p4-branch.sh   |   48 +++++++++++++++++++++++++++++
 t/t9804-git-p4-label.sh    |   73 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+), 28 deletions(-)
 create mode 100755 t/t9804-git-p4-label.sh

-- 
1.7.7.295.g34dd4

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jonathan Nieder @ 2011-11-07 21:32 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, git, Jim Meyering,
	Fredrik Gustafsson, Andreas Schwab
In-Reply-To: <7vlirr1vi5.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> [jn: drop the GREP_HEADER_FIELD_MAX <= p->field check, too,
>>  for symmetry]
>
> Umm, why is this removal of defensive programming practice an improvement?

Checking "p->filed < 0" makes static analyzers complain.  There's no
clean way I know of[*] to get them to shut up and keep the check.  The
fundamental question is whether the -Wtype-limits check is worth it or
not:

 - on one hand, it catches real bugs, such as the mistaken check for
   negative size_t Ævar mentioned;

 - on the other hand, it trips for cases like this in which the type
   only happened to be unsigned on the build platform.  I consider
   that a gcc bug (and apparently clang shares it) --- see
   <http://bugs.debian.org/615525>.

So, the purpose of this patch was to work around this common bug in
static analyzers.

Checking "GREP_HEADER_FIELD_MAX <= p->field" without checking for
"p->field < 0", like the original patch did, would be only checking
half of what it intends to and not add any real guarantees.  And
more importantly, it would be distracting to people like me and
Andreas who would wonder "why doesn't this check p->field < 0, too?".

I guess the commit message should have said

	grep: remove a defensive check to work around common static analyzer bug

> This part is mostly my code and because I know what I wrote is almost
> always perfect, so I do not think there is any real harm done, but still...

Heh.

[*] There are plenty of cryptic, hackish ways possible, though. :)

	if ((size_t) p->field >= GREP_HEADER_FIELD_MAX)
		die(...);

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Jeff King @ 2011-11-07 21:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vhb2f1v7g.fsf@alter.siamese.dyndns.org>

On Mon, Nov 07, 2011 at 01:25:23PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > That makes sense. But I think it fits in with git's current UI to do
> > this via a combination of push options and refspecs. Even if we want to
> > wrap it in some "git remote" command for convenience, I think what
> > you're asking should be implemented as part of "git push".
> 
> Yeah, I think it makes sense to give --prune to "push" just like "fetch"
> already has. These two are the primary (and in the ideal world, only)
> operations that talk to the outside world. "remote add -f" might have been
> a tempting "convenience" feature, but I personally think it probably was a
> mistake for the exact reason that letting anything but "push" and "fetch"
> talk to the outside world just invites more confusion. There does not have
> to be 47 different ways to do the same thing.

I don't mind "add -f" too much, which is at least very clear that it is
simply a convenience feature for "git remote add foo && git fetch foo".
But the other "git remote" features like "set-head -a", which can't be
done any other way, or the "auto-check-what-the-remote-has" feature of
"git remote show" are a little gross.

Anyway, I think we are on the same page; this feature (and btw, I think
this is a great feature that we should have) should go into "push".

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Junio C Hamano @ 2011-11-07 21:25 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, git
In-Reply-To: <20111107210134.GA7380@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> That makes sense. But I think it fits in with git's current UI to do
> this via a combination of push options and refspecs. Even if we want to
> wrap it in some "git remote" command for convenience, I think what
> you're asking should be implemented as part of "git push".

Yeah, I think it makes sense to give --prune to "push" just like "fetch"
already has. These two are the primary (and in the ideal world, only)
operations that talk to the outside world. "remote add -f" might have been
a tempting "convenience" feature, but I personally think it probably was a
mistake for the exact reason that letting anything but "push" and "fetch"
talk to the outside world just invites more confusion. There does not have
to be 47 different ways to do the same thing.

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Junio C Hamano @ 2011-11-07 21:18 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Ævar Arnfjörð Bjarmason, git, Jim Meyering,
	Fredrik Gustafsson
In-Reply-To: <20111107194912.GA12469@elie.hsd1.il.comcast.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> [jn: drop the GREP_HEADER_FIELD_MAX <= p->field check, too,
>  for symmetry]

Umm, why is this removal of defensive programming practice an improvement?
This part is mostly my code and because I know what I wrote is almost
always perfect, so I do not think there is any real harm done, but still...

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  grep.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index b29d09c7..424c46cd 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -327,8 +327,6 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
>  	for (p = opt->header_list; p; p = p->next) {
>  		if (p->token != GREP_PATTERN_HEAD)
>  			die("bug: a non-header pattern in grep header list.");
> -		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
> -			die("bug: unknown header field %d", p->field);
>  		compile_regexp(p, opt);
>  	}

^ permalink raw reply

* Re: pretty placeholders for reflog entries
From: Jeff King @ 2011-11-07 21:13 UTC (permalink / raw)
  To: Jack Nagel; +Cc: git
In-Reply-To: <CAMYxyaWPWVGUHz0qQOnARb9wexHCx73a04Bu_UhrJR=xrinX7g@mail.gmail.com>

On Sun, Nov 06, 2011 at 11:49:48PM -0600, Jack Nagel wrote:

> I have the reflog enabled on a bare repo so that I can have a record of
> "who pushed what, when". I'd like to define a custom pretty format for
> use with "git log -g" for reading it, but unfortunately the placeholders
> for reflog information are somewhat limited. In particular, I'd like to
> be able to access the identity (i.e., name and email) and date from each
> reflog entry.
> 
> Is it possible to extract this information in current git? Perhaps I
> overlooked something.

Sorry, but there aren't convenient placeholders for that. There probably
should be.

As a workaround, you can get the reflog selector ("%gD"), and then
cross-reference it with the output of "git log -g". But obviously that's
inefficient and a giant pain. We really should have "%gn" and "%ge" for
the reflog name and email. And related date options, though annoyingly
"%gd" is already taken for the default format.

I doubt it would be a very big patch. Want to get your feet wet in git
development? :)

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Jeff King @ 2011-11-07 21:01 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s0M-qnZeHCUadSJJCYO=t881sUOi11G3fCG2vaAakPyBQ@mail.gmail.com>

On Mon, Nov 07, 2011 at 10:51:10PM +0200, Felipe Contreras wrote:

> > What I don't understand is why it is not:
> >
> >  git push --mirror <URL|remote>
> 
> Because that pushes *everything*.

Ahh, I think I see. It is doing --mirror, but only on a reduced refspec?

In that case, is there a reason that:

  git push --prune <URL|remote> refs/heads/*

would not do what you want (note that "--prune" does not exist, but I
think it should).

> > That's what I don't understand from your proposal. Your command is just
> > pushing something to the remote, right? Why isn't "push" the command,
> > and your sync options become options to push?
> 
> How exactly? --sync-prune, --sync-new, --sync-all? But actually, I was
> thinking on adding an option to sync the other way around; to get all
> the remote branches and have them locally.

If I understand correctly, you have three modes:

  1. update remote refs with local values, prune anything remote that we
     don't have locally (--sync-prune)

  2. update remote refs with local values, including pushing anything
     new that we don't have locally (--sync-new)

  3. push new and prune (i.e., 1 and 2 together)

If we had "git push --prune" as above, those would be:

  1. git push --prune <remote> :

     I.e., use the "matching" refspec to not push new things, but turn
     on pruning.

  2. git push <remote> refs/heads/*

     Turn off pruning, but use an explicit refspec, not just "matching",
     which will push all local branches.

  3. git push --prune <remote> refs/heads/*

     Turn on both features.

> Well, I usually have quite a lot of branches in my local repositories,
> like a dozen of so. And I like to back them up in some remote
> repository, however, not all the branches all the time. git push
> --mirror not only pushes branches, but also tags (and I don't want
> that), and even other refs. Does that clarifies things?

That makes sense. But I think it fits in with git's current UI to do
this via a combination of push options and refspecs. Even if we want to
wrap it in some "git remote" command for convenience, I think what
you're asking should be implemented as part of "git push".

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Felipe Contreras @ 2011-11-07 20:51 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111107183938.GA5155@sigill.intra.peff.net>

On Mon, Nov 7, 2011 at 8:39 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Nov 07, 2011 at 08:35:07PM +0200, Felipe Contreras wrote:
>
>> I don't know, seems logical to me what 'git remote sync' does, but
>> 'git push sync'? That sounds weird, and there are no 'git push foo'
>> commands.
>
> What I don't understand is why it is not:
>
>  git push --mirror <URL|remote>

Because that pushes *everything*.

>> > And how does this differ from "git push --mirror"? It looks like you
>> > have more options for what pushing all versus pruning, but wouldn't it
>> > be better for "git push" to grow those options?
>>
>> But how? --mirror is just an option, I want a separate command, with
>> it's own options.
>
> That's what I don't understand from your proposal. Your command is just
> pushing something to the remote, right? Why isn't "push" the command,
> and your sync options become options to push?

How exactly? --sync-prune, --sync-new, --sync-all? But actually, I was
thinking on adding an option to sync the other way around; to get all
the remote branches and have them locally.

> Can you step back and describe the problem you're trying to solve? Maybe
> we're not connecting there.

Well, I usually have quite a lot of branches in my local repositories,
like a dozen of so. And I like to back them up in some remote
repository, however, not all the branches all the time. git push
--mirror not only pushes branches, but also tags (and I don't want
that), and even other refs. Does that clarifies things?

-- 
Felipe Contreras

^ permalink raw reply

* Re: BUG. Git config pager when --edit
From: Frans Klaver @ 2011-11-07 20:45 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King; +Cc: Alexey Shumkin, git
In-Reply-To: <20111107171800.GA3621@sigill.intra.peff.net>

On Mon, 07 Nov 2011 18:18:00 +0100, Jeff King <peff@peff.net> wrote:

>> I was actually hoping that you won't go that route, but the route to  
>> push
>> further to decide/spawn pager as late as possible. Clearly no sane  
>> person
>> would want to run --edit subcommand under pager and "pager.config =  
>> less"
>> should just be ignored in such a case.
>
> The problem with that is that it dumps the responsibility for running
> the pager to every subcommand. For builtins, we can have a flag that
> says "respect the pager.log config" or "foo will handle this itself;
> don't respect pager.tag".
>
> But what about externals? If "pager.stash" does nothing in git.c, and
> leaves it to "git-stash.sh" to start the pager if and when it's
> appropriate, then what about my personal "git-foo" that I drop into my
> PATH? Now I can't use "config.foo" without carrying code to do so in my
> external command.
>
> Maybe that's an OK tradeoff. But it's more of a pain for existing
> scripts, and it's not backwards compatible. What do you think?

For both cases there's something to say. In any new design I might dump  
the responsibility on the external, but I would prefer to keep the  
decision logic centralized. But as I understand, removing the  
responsibility from git.c is going to require a whole bunch of other  
changes to get the pager functional again in the scripts. So if there is a  
somewhat decent way to be sure about whether or not to use the pager (i.e.  
no editing) in git.c, why not keep it there? If, on the other hand, the  
code is going to turn out to be a big hack, I'd say move it out.

Frans

^ permalink raw reply

* Re: A Python script to put CTAN into git (from DVDs)
From: Jonathan Fine @ 2011-11-07 20:21 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: python-list, git
In-Reply-To: <m3zkg92dxq.fsf@localhost.localdomain>

On 06/11/11 20:28, Jakub Narebski wrote:

> Note that for gitPAN each "distribution" (usually but not always
> corresponding to single Perl module) is in separate repository.
> The dependencies are handled by CPAN / CPANPLUS / cpanm client
> (i.e. during install).

Thank you for your interest, Jakub, and also for this information.  With 
TeX there's a difficult which Perl, I think, does not have.  With TeX we 
process documents, which may demand specific versions of packages. 
LaTeX users are concerned that move on to a later version will cause 
documents to break.

> Putting all DVD (is it "TeX Live" DVD by the way?) into single
> repository would put quite a bit of stress to git; it was created for
> software development (although admittedly of large project like Linux
> kernel), not 4GB+ trees.

I'm impressed by how well git manages it.  It took about 15 minutes to 
build the 4GB tree, and it was disk speed rather than CPU which was the 
bottleneck.

>> Once you've done that, it is then possible and sensible to select
>> suitable interesting subsets, such as releases of a particular
>> package. Users could even define their own subsets, such as "all
>> resources needed to process this file, exactly as it processes on my
>> machine".
>
> This could be handled using submodules, by having superrepository that
> consist solely of references to other repositories by the way of
> submodules... plus perhaps some administrativa files (like README for
> whole CTAN, or search tool, or DVD install, etc.)
>
> This could be the used to get for example contents of DVD from 2010.

We may be at cross purposes.  My first task is get the DVD tree into 
git, performing necessary transformations such as expanding zip files 
along the way.  Breaking the content into submodules can, I believe, be 
done afterwards.

With DVDs from several years it could take several hours to load 
everything into git.  For myself, I'd like to do that once, more or less 
as a batch process, and then move on to the more interesting topics. 
Getting the DVD contents into git is already a significant piece of work.

Once done, I can them move on to what you're interested in, which is 
organising the material.  And I hope that others in the TeX community 
will get involved with that, because I'm not building this repository 
just for myself.

> But even though submodules (c.f. Subversion svn:external, Mecurial
> forest extension, etc.) are in Git for quite a bit of time, it doesn't
> have best user interface.
>
>> In addition, many TeX users have a TeX DVD.  If they import it into a
>> git repository (using for example my script) then the update from 2011
>> to 2012 would require much less bandwidth.
>
> ???

A quick way to bring your TeX distribution up to date is to do a delta 
with a later distribution, and download the difference.  That's what git 
does, and it does it well.  So I'm keen to convert a TeX DVD into a git 
repository, and then differences can be downloaded.

>> Finally, I'd rather be working within git that modified copy of the
>> ISO when doing the subsetting.  I'm pretty sure that I can manage to
>> pull the small repositories from the big git-CTAN repository.
>
> No you cannot.  It is all or nothing; there is no support for partial
> _clone_ (yet), and it looks like it is a hard problem.
>
> Nb. there is support for partial _checkout_, but this is something
> different.

 From what I know, I'm confident that I can achieve what I want using 
git.  I'm also confident that my approach is not closing off any 
possible approached.  But if I'm wrong you'll be able to say: I told you so.

> Commit = tree + parent + metadata.

Actually, any number of parents, including none.  What metadata do I 
have to provide?  At this time nothing, I think, beyond that provided by 
the name of a reference (to the root of a tree).

> I think you would very much want to have linear sequence of trees,
> ordered via DAG of commits.  "Naked" trees are rather bad idea, I think.
>
>> As I recall the first 'commit' to the git repository for the Linux
>> kernel was just a tree, with a reference to that tree as a tag.  But
>> no commit.
>
> That was a bad accident that there is a tag that points directly to a
> tree of _initial import_, not something to copy.

Because git is a distributed version control system, anyone who wants to 
can create such a directed acyclic graph of commits.  And if it's useful 
I'll gladly add it to my copy of the repository.

best regards


Jonathan

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Andreas Schwab @ 2011-11-07 20:13 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
	Jim Meyering, Fredrik Gustafsson
In-Reply-To: <20111107185536.GA5450@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I do still question the value of the check at all, though, given that
> static analysis can find those bugs.

Then you should remove the whole statement.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Jonathan Nieder @ 2011-11-07 19:49 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Jim Meyering, Fredrik Gustafsson
In-Reply-To: <1320581184-4557-4-git-send-email-avarab@gmail.com>

Hi,

> --- a/grep.c
> +++ b/grep.c
> @@ -327,7 +327,7 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
>  	for (p = opt->header_list; p; p = p->next) {
>  		if (p->token != GREP_PATTERN_HEAD)
>  			die("bug: a non-header pattern in grep header list.");
> -		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
> +		if (GREP_HEADER_FIELD_MAX <= p->field)

I imagine the following would be less controversial.

-- >8 --
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Subject: grep: get rid of bounds check on "enum grep_header_field" value

The grep_header_field enum is defined as:

    enum grep_header_field {
    	GREP_HEADER_AUTHOR = 0,
    	GREP_HEADER_COMMITTER
    };

Git's grep code sanity-checks the value of p->field before using it in
order to avoid reading and writing past the end of an array.
Unfortunately that test trips up misguided static analyzers like
"gcc -Wtype-limits" that do not recognize that C allows this enum to
be a signed integer type and an "x < 0" comparison really could fire
if someone passed in an uninitialized value.

Let's just drop the check.  Luckily git always does initialize
p->field correctly, and if it were to stop doing so, then hopefully
running the test suite with valgrind would catch it.

Noticed with clang -Wtautological-compare.

[jn: drop the GREP_HEADER_FIELD_MAX <= p->field check, too,
 for symmetry]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 grep.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index b29d09c7..424c46cd 100644
--- a/grep.c
+++ b/grep.c
@@ -327,8 +327,6 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 	for (p = opt->header_list; p; p = p->next) {
 		if (p->token != GREP_PATTERN_HEAD)
 			die("bug: a non-header pattern in grep header list.");
-		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
-			die("bug: unknown header field %d", p->field);
 		compile_regexp(p, opt);
 	}
 
-- 
1.7.8.rc0

^ permalink raw reply related

* Re: hook for rebase --continue
From: Martin Fick @ 2011-11-07 19:46 UTC (permalink / raw)
  To: Matt Graham; +Cc: git
In-Reply-To: <CALts4TQ545L1d1J0EiUjd7x=WBJpjCCv6UsXZOoGQAC29RqC5g@mail.gmail.com>

On Monday, November 07, 2011 12:42:32 pm Matt Graham wrote:
> Does anyone else share my expectation that the pre-commit
> hook should run during a rebase? Or at least for the
> first commit following a rebase conflict?

I have had the same concern.  We use a change-id hook for 
Gerrit development, and it adds a footer line to commit 
messages.  If during a rebase, it is accidentally removed, 
the hook does not get run and does not re-add it,

-Martin

-- 
Employee of Qualcomm Innovation Center, Inc. which is a 
member of Code Aurora Forum

^ permalink raw reply

* hook for rebase --continue
From: Matt Graham @ 2011-11-07 19:42 UTC (permalink / raw)
  To: git

Hi,
I did some testing and it appears that during a rebase, if I resolve a
conflict and call git rebase --continue, the pre-commit hook doesn't
run.  This means that if I don't resolve the conflict correctly, our
check for invalid syntax doesn't get run and creates the risk that
someone could push code with invalid syntax, not realizing that the
check didn't run.

Does anyone else share my expectation that the pre-commit hook should
run during a rebase? Or at least for the first commit following a
rebase conflict?

If not, is there another hook that is triggered by a rebase that I
should be using instead?

Thanks,
Matt

^ permalink raw reply

* Re: [PATCH 1/3] apply: get rid of useless x < 0 comparison on a size_t type
From: Giuseppe Bilotta @ 2011-11-07 19:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð, git, Jim Meyering,
	Fredrik Gustafsson
In-Reply-To: <7vsjm15cap.fsf@alter.siamese.dyndns.org>

On Sun, Nov 6, 2011 at 7:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>> This check was originally added in v1.6.5-rc0~53^2 by Giuseppe Bilotta
>> while adding an option to git-apply to ignore whitespace differences.
>
> I agree that these quantities can never be negative, so I'll apply the
> patch as is.

I agree too. In fact I spotted this some time ago when I started
compiling git with Clang but never found the time to look into it.

> But I have this suspicion that this was a rather sloppy defensive check to
> protect this codepath against potential breakage in another codepath (most
> likely update_pre_post_images() touched by the same patch) that adjusts
> image->line[].len the caller of this function uses to feed these two
> parameters. Giuseppe may have been not confident enough that the code
> added to that function ensures not to undershoot when it reduces "len", or
> something.
>
> Giuseppe, can you explain what is going on?

No, I can't, so I guess the sloppy coding is the right motivation. I
remember working this patch from a rather older submission that was
never followed-up to, so my guess is that I just forgot to clean it up
properly, and during review the focus was obviously on other aspects
of the submission too. Also, having a look at the current caller of
the function, I don't see how the check would even be needed.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: Is there a pdf git manual?
From: Peng Yu @ 2011-11-07 19:07 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3obwn3gtz.fsf@localhost.localdomain>

Hi Jakub,

> "make pdf" would generate PDF version of (some of) documentation.

I get the pdf generated. When you say 'some of", what is missing from the pdf?


-- 
Regards,
Peng

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Ævar Arnfjörð Bjarmason @ 2011-11-07 19:06 UTC (permalink / raw)
  To: Jeff King
  Cc: Andreas Schwab, git, Junio C Hamano, Jim Meyering,
	Fredrik Gustafsson
In-Reply-To: <20111107185536.GA5450@sigill.intra.peff.net>

On Mon, Nov 7, 2011 at 19:55, Jeff King <peff@peff.net> wrote:

> I do still question the value of the check at all, though, given that
> static analysis can find those bugs.

Agreed, which is why I submitted this patch.

Also if this is the sort of thing we'd like to guard against we should
be discussing it more generally. We have a bunch of occurances where
we'd probably break down if someone manually assigned -1 to an enum
variable:

    $ git grep -E -A1 'enum.*\{' | grep -B1 '=.*0' | grep enum
    builtin/branch.c:enum color_branch {
    builtin/branch.c:static enum merge_filter {
    builtin/fetch-pack.c:enum ack_type {
    builtin/fetch.c:enum {
    builtin/grep.c: enum {
    builtin/mv.c:   enum update_mode { BOTH = 0, WORKING_DIRECTORY,
INDEX } *modes;
    builtin/remote.c:enum {
    builtin/remote.c:       enum {
    cache.h:enum rebase_setup_type {
    cache.h:enum push_default_type {
    cache.h:enum object_creation_mode {
    cache.h:enum sharedrepo {
    cache.h:enum date_mode {
    cache.h:        enum {
    convert.h:enum safe_crlf {
    convert.h:enum auto_crlf {
    diff.h:enum diff_words_type {
    diff.h:enum color_diff {
    dir.c:enum exist_status {
    dir.h:  enum {
    grep.h:enum grep_header_field {
    http-push.c:enum dav_header_flag {
    imap-send.c:enum CAPABILITY {
    log-tree.c:enum decoration_type {
    merge-recursive.c:enum rename_type {
    merge-recursive.h:      enum {
    notes-merge.h:  enum {
    remote.h:enum match_refs_flags {
    rerere.c:       enum {
    unpack-trees.h:enum unpack_trees_error_types {
    wt-status.h:enum color_wt_status {

^ permalink raw reply


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