Git development
 help / color / mirror / Atom feed
* Re: Millisecond precision in timestamps?
From: Felipe Contreras @ 2012-11-28  3:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric S. Raymond, Shawn Pearce, Junio C Hamano, git
In-Reply-To: <20121128033009.GA3931@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 4:30 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 27, 2012 at 10:23:37PM -0500, Eric S. Raymond wrote:
>
>> Jeff King <peff@peff.net>:
>> > But I really wonder if anybody actually cares about adding sub-second
>> > timestamp support, or if it is merely "because SVN has it".
>>
>> There's actually one possible other reason to care.  1-second granularity
>> isn't quite fine enough to guarantee that a (committer, timestamp)
>> pair is a unique key.  1 microsecond granularity would be.
>
> You can't guarantee that such a pair is unique, anyway, due to clock
> skew.
>
> A much more compelling argument to me would be that you are doing some
> bidirectional magic between git and svn, and you want to make make sure
> that an svn->git->svn translation will result in the exact same bytes.
> Then the argument is still "because SVN has it", but at least it is "and
> we interoperate with it" and not simply chasing a cool but useless
> feature.

But the same can be said of mercurial and bzr. This can be solved
attaching some external SCM information in notes, and somehow make
fast-export throw that info along with the commit.

For now the solution has been to append the extra information into the
commit message, which is ugly and hacky, but it works.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH] Third try at documenting command integration requirements.
From: Eric S. Raymond @ 2012-11-28  3:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Haggerty, git
In-Reply-To: <7vobiijxol.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com>:
> I won't worry about Python 3 yet; in what timeframe did Python's
> i18n/unicode support become usable?  In 2.4, or 2.6?

Er, it depends on what you consider "usable".

Unicode integration turned out to have a lot messier edge cases than
anyone understood going in.  First-cut support was in 1.6, but I'd say
it still has some pretty sharp edges *today*.  Which is why 3.0 has
gone all-Unicode-all-the-time.  The problems mostly come from having
two different notions of "string" that don't really mix well.

Me, I still avoid the hell out of Unicode in Python.  And occasionally
fund myself cursing a library maintainer who didn't.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: git config key bug or by design?
From: Jeff King @ 2012-11-28  3:34 UTC (permalink / raw)
  To: Peter van der Does; +Cc: git
In-Reply-To: <20121127221446.7f2fbf71@Indy>

On Tue, Nov 27, 2012 at 10:14:46PM -0500, Peter van der Does wrote:

> I noticed today I can't create a key starting with a number.
> 
> The source code[1] confirms this, but is this a bug or is it by design?

I don't recall ever discussing it. But what is it that you want to store
in a key starting with a number? Git does not respect any such config
values[1].

Are you writing a new tool that will store its config alongside git's?
Even if the behavior is loosened, you would probably want to avoid
starting your config keys with numbers, as older git versions would be
around for a while and would choke on it.

-Peff

[1] You can still store arbitrary bytes in the subsection name (e.g.,
    "foo.123.bar").

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Felipe Contreras @ 2012-11-28  3:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20121128032245.GD27772@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 4:22 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:
>
>> > We could improve the test in t5801, but it is nice to let people on such
>> > systems test it, as well. And the infrastructure might be useful if we
>> > ever acquire more bash scripts.
>> >
>> > There's a fair bit of boilerplate, but I think this squashable patch
>> > would do it:
>>
>> Yeah, but I wonder what's the point of installing this script, it's
>> mostly for testing and reference, and to add a whole category for that
>> seems like overkill.
>
> There's no point in installing it; I just didn't make the effort to
> avoid doing so (note that testpy and testsvn are also installed, which
> are in the same boat; it might make sense to split them all out like we
> do for $TEST_PROGRAMS).
>
> I agree it's an annoying amount of boilerplate, but it seems simpler
> cognitively to me for it to behave as the other SCRIPT_* builds than to
> do something simple but inconsistent.
>
> I do not care enough to argue about it. We need to do something to fix
> the impending test breakage on systems like Solaris. I have posted the
> patch to handle BASH_PATH, so do what you want.

I'm not objecting to the change, I'm simply wondering.

Personally I think switching to '/usr/bin/env bash' should be enough
for now. Doing a grep on my git installation throws this:

% grep '/usr/bin/env' -r /opt/git
/opt/git/lib/python2.7/site-packages/git_remote_helpers/git/git.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/__init__.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/util.py:#!/usr/bin/env
python
/opt/git/libexec/git-core/git-instaweb:#!/usr/bin/env ruby

So it's doubtful a lack of /usr/bin/env would cause any more breakages
than it already does for test-git.

And this just landed on 'pu', I don't think there's anything big
impending. If the /usr/bin/env solution turned out to be not enough
for some reason, we can deal with it then. That's my opinion.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Millisecond precision in timestamps?
From: Jeff King @ 2012-11-28  3:30 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: Shawn Pearce, Felipe Contreras, Junio C Hamano, git
In-Reply-To: <20121128032337.GB1669@thyrsus.com>

On Tue, Nov 27, 2012 at 10:23:37PM -0500, Eric S. Raymond wrote:

> Jeff King <peff@peff.net>:
> > But I really wonder if anybody actually cares about adding sub-second
> > timestamp support, or if it is merely "because SVN has it".
> 
> There's actually one possible other reason to care.  1-second granularity 
> isn't quite fine enough to guarantee that a (committer, timestamp)
> pair is a unique key.  1 microsecond granularity would be.

You can't guarantee that such a pair is unique, anyway, due to clock
skew.

A much more compelling argument to me would be that you are doing some
bidirectional magic between git and svn, and you want to make make sure
that an svn->git->svn translation will result in the exact same bytes.
Then the argument is still "because SVN has it", but at least it is "and
we interoperate with it" and not simply chasing a cool but useless
feature.

-Peff

^ permalink raw reply

* [PATCH] svnrdump_sim: start the script with /usr/bin/env python
From: Christian Couder @ 2012-11-28  2:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

All the python scripts except contrib/svn-fe/svnrdump_sim.py
start with "#!/usr/bin/env python".

This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 contrib/svn-fe/svnrdump_sim.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..d219180 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 """
 Simulates svnrdump by replaying an existing dump from a file, taking care
 of the specified revision range.
-- 
1.7.11.rc3.17.g55b3f8c

^ permalink raw reply related

* Re: Millisecond precision in timestamps?
From: Eric S. Raymond @ 2012-11-28  3:23 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn Pearce, Felipe Contreras, Junio C Hamano, git
In-Reply-To: <20121128011750.GA23498@sigill.intra.peff.net>

Jeff King <peff@peff.net>:
> But I really wonder if anybody actually cares about adding sub-second
> timestamp support, or if it is merely "because SVN has it".

There's actually one possible other reason to care.  1-second granularity 
isn't quite fine enough to guarantee that a (committer, timestamp)
pair is a unique key.  1 microsecond granularity would be.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-28  3:22 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <CAMP44s3ZtCNsedJtsHDJx5d4Myjbbp+D6yT-rO-CmKOTtt91VQ@mail.gmail.com>

On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:

> > We could improve the test in t5801, but it is nice to let people on such
> > systems test it, as well. And the infrastructure might be useful if we
> > ever acquire more bash scripts.
> >
> > There's a fair bit of boilerplate, but I think this squashable patch
> > would do it:
> 
> Yeah, but I wonder what's the point of installing this script, it's
> mostly for testing and reference, and to add a whole category for that
> seems like overkill.

There's no point in installing it; I just didn't make the effort to
avoid doing so (note that testpy and testsvn are also installed, which
are in the same boat; it might make sense to split them all out like we
do for $TEST_PROGRAMS).

I agree it's an annoying amount of boilerplate, but it seems simpler
cognitively to me for it to behave as the other SCRIPT_* builds than to
do something simple but inconsistent.

I do not care enough to argue about it. We need to do something to fix
the impending test breakage on systems like Solaris. I have posted the
patch to handle BASH_PATH, so do what you want.

-Peff

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Felipe Contreras @ 2012-11-28  3:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20121128031118.GB27772@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 4:11 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:
>
>> > >  Renaming of remote-testgit feels to be a mistake.  It probably
>> > >  should keep its source in remote-testgit.bash and generate it,
>> >
>> > Why generate it? There's nothing to generate. python's source code
>> > needs regeneration, bash's code doesn't.
>>
>> We fix up the #!-lines on all of the existing shell scripts (as well as
>> python and perl). Wouldn't we want to do the same for people who have
>> bash in an alternate location?
>>
>> As the series is now, people with bash in their PATH, but not in
>> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
>> git-remote-testgit hardcodes /bin/bash).
>
> We could improve the test in t5801, but it is nice to let people on such
> systems test it, as well. And the infrastructure might be useful if we
> ever acquire more bash scripts.
>
> There's a fair bit of boilerplate, but I think this squashable patch
> would do it:

Yeah, but I wonder what's the point of installing this script, it's
mostly for testing and reference, and to add a whole category for that
seems like overkill.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* git config key bug or by design?
From: Peter van der Does @ 2012-11-28  3:14 UTC (permalink / raw)
  To: git

I noticed today I can't create a key starting with a number.

The source code[1] confirms this, but is this a bug or is it by design?


[1]: https://github.com/git/git/blob/master/config.c#L1265

-- 
Peter van der Does

GPG key: CB317D6E

IRC: Ganseki on irc.freenode.net
Twitter: @petervanderdoes

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-28  3:12 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <CAMP44s3F4MPm-kLAW67rZG8oZg76CGS32h44LnK05UT11TOnSA@mail.gmail.com>

On Wed, Nov 28, 2012 at 04:10:07AM +0100, Felipe Contreras wrote:

> >> Why generate it? There's nothing to generate. python's source code
> >> needs regeneration, bash's code doesn't.
> >
> > We fix up the #!-lines on all of the existing shell scripts (as well as
> > python and perl). Wouldn't we want to do the same for people who have
> > bash in an alternate location?
> 
> '#!/usr/bin/env bash' should take care of people who have bash in an
> alternate location, no?

If that is where there env is. I do not recall offhand if that is a
problem for anyone. Still, I don't think it is that big a deal to treat
it like we do other scripts, and provide the usual boilerplate (which
also gets us NO_BASH if you have an old or broken bash and want to skip
the tests). I just sent a patch.

-Peff

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-28  3:11 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <20121128025928.GA27772@sigill.intra.peff.net>

On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:

> > >  Renaming of remote-testgit feels to be a mistake.  It probably
> > >  should keep its source in remote-testgit.bash and generate it,
> > 
> > Why generate it? There's nothing to generate. python's source code
> > needs regeneration, bash's code doesn't.
> 
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?
> 
> As the series is now, people with bash in their PATH, but not in
> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
> git-remote-testgit hardcodes /bin/bash).

We could improve the test in t5801, but it is nice to let people on such
systems test it, as well. And the infrastructure might be useful if we
ever acquire more bash scripts.

There's a fair bit of boilerplate, but I think this squashable patch
would do it:

diff --git a/.gitignore b/.gitignore
index 46595db..3b636bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -124,6 +124,7 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
+/git-remote-testgit
 /git-remote-testsvn
 /git-remote-testpy
 /git-repack
diff --git a/Makefile b/Makefile
index 68f1ac2..19f6fd2 100644
--- a/Makefile
+++ b/Makefile
@@ -424,6 +424,7 @@ PROGRAM_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
+SCRIPT_BASH =
 SCRIPT_SH =
 SCRIPT_LIB =
 TEST_PROGRAMS_NEED_X =
@@ -473,9 +474,12 @@ SCRIPT_PERL += git-svn.perl
 SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
+SCRIPT_BASH += git-remote-testgit.bash
+
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
+	  $(patsubst %.bash,%,$(SCRIPT_BASH)) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS
@@ -571,9 +575,13 @@ endif
 ifndef PYTHON_PATH
 	PYTHON_PATH = /usr/bin/python
 endif
+ifndef BASH_PATH
+	BASH_PATH = /bin/bash
+endif
 
 export PERL_PATH
 export PYTHON_PATH
+export BASH_PATH
 
 LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
@@ -1931,6 +1939,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON=NoThanks
 endif
 
+ifeq ($(BASH_PATH),)
+NO_BASH=NoThanks
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -2006,6 +2018,7 @@ gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
+BASH_PATH_SQ = $(subst ','\'',$(BASH_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 
@@ -2267,6 +2280,15 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
 	mv $@+ $@
 endif # NO_PYTHON
 
+ifndef NO_BASH
+$(patsubst %.bash,%,$(SCRIPT_BASH)):
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/bash|#!$(BASH_PATH_SQ)|' \
+	    $@.bash >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif
+
 configure: configure.ac GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $<+ && \
 	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -2599,6 +2621,7 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+	@echo NO_BASH=\''$(subst ','\'',$(subst ','\'',$(NO_BASH)))'\' >>$@
 	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
 ifdef GIT_TEST_OPTS
 	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
diff --git a/git-remote-testgit b/git-remote-testgit.bash
similarity index 100%
rename from git-remote-testgit
rename to git-remote-testgit.bash
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 456303b..d6ebc5e 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -7,7 +7,7 @@ test_description='Test remote-helper import and export commands'
 
 . ./test-lib.sh
 
-if ! type "${BASH-bash}" >/dev/null 2>&1; then
+if ! test_have_prereq BASH; then
 	skip_all='skipping remote-testgit tests, bash not available'
 	test_done
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..5300fa9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -675,6 +675,7 @@ esac
 ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
+test -z "$NO_BASH" && test_set_prereq BASH
 test -n "$USE_LIBPCRE" && test_set_prereq LIBPCRE
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 

^ permalink raw reply related

* Re: Topics currently in the Stalled category
From: Felipe Contreras @ 2012-11-28  3:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20121128025928.GA27772@sigill.intra.peff.net>

On Wed, Nov 28, 2012 at 3:59 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:
>
>> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Here is a list of stalled topics I am having trouble deciding what
>> > to do (the default is to dismiss them around feature freeze).
>> >
>> > * fc/fast-export-fixes (2012-11-08) 14 commits
>> >
>> >  Renaming of remote-testgit feels to be a mistake.  It probably
>> >  should keep its source in remote-testgit.bash and generate it,
>>
>> Why generate it? There's nothing to generate. python's source code
>> needs regeneration, bash's code doesn't.
>
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?

'#!/usr/bin/env bash' should take care of people who have bash in an
alternate location, no?

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v3 0/7] New remote-bzr remote helper
From: Felipe Contreras @ 2012-11-28  3:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vpq2yihaq.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 3:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> At this point, both have been cooking for a week or more in 'next',
>>> there is no existing users, they are on the fringe so breakages in
>>> them won't negatively affect anybody anyway.  So it doesn't matter
>>> much if they are merged to 'master' and then fixed up with follow up
>>> patches after that, or fixed up with follow up patches while they
>>> are in 'next', as they won't be rewound and restarted from scratch.
>>
>> The fixes are affecting some people, that's why I did them. Some were
>> reported here in the mailing list, and some in my github's clone:
>>
>> https://github.com/felipec/git/issues?page=1&state=closed
>
> Are you talking about -hg or -bzr or both?
>
> In any case, I am mostly concerned about *my* next release, whose
> rc0 will be tagged sometime this week or the next week.
>
> People who have been bitten by bugs from *your* tree or versions in
> 'next' do not count.  When I said "no existing users", I was talking
> about the end users who need rock solid stable "releases" because
> tagged versions are the only ones they use.

If users you call "fringe" have noticed these compatibility issues,
chances are your "existing users" are going to catch them as well.

Those issues were fixed right away, but I didn't sent them because I
wanted things to settle. I didn't see that v2 landed in next until
now.

> If the code of these topics is still in flux and needs constant
> fixes, probably it is a better idea to cook them longer in 'next',
> skipping the upcoming 1.8.1 release.  If we are going to go that
> route, we can drop the v2 fc/remote-bzr and queue v3 when we rewind
> the tip of 'next' after 1.8.1 release (by that time you may have v4
> of the series, but then we can skip v3).  Is that more preferrable
> than rushing these topics forward before they are ready for general
> audience?

They are not in constant flux, that's why I haven't send any new
re-rolls since v3, which was on November 11. I've been using v3 for
baseline since them, and the rest of the patches I've sent on top of
that.

In fact, these particular fixes were already sent on November 13 (on top of v3):
http://article.gmane.org/gmane.comp.version-control.git/209558

On November 10 Jeff threatened to to merge v2 to next on the "What's
cooking", and I told him I was about to sent a re-roll, he
acknowledged the same day, and I sent the next one.

Since v3 remote-bzr hasn't been on flux.

Now, what you do is up to you, but I think v3 plus the two patches I
sent on nov 13, and just resent today should be safe. That being said,
I don't use remote-bzr really, and I don't know how many people have
been using it, so I have no idea how ready it really is. If I were you
I would just merge v3 to next, or revert v2 and merge v3, and then
apply the two patches on top. Or if you want, revert v2, wait for
1.8.1, and then merge v3. Either way it's doubtful there will be a v4
(if there are next patches they will be on top of v3, as they have
been for quite some time now), so it's not clear what "existing users"
will gain by that.

I'm confident about remote-hg though.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Topics currently in the Stalled category
From: Jeff King @ 2012-11-28  2:59 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git
In-Reply-To: <CAMP44s30cbH5+HUxRBByk5sZGq=j_MdqLSnNzREozEk40_zbOw@mail.gmail.com>

On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:

> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Here is a list of stalled topics I am having trouble deciding what
> > to do (the default is to dismiss them around feature freeze).
> >
> > * fc/fast-export-fixes (2012-11-08) 14 commits
> >
> >  Renaming of remote-testgit feels to be a mistake.  It probably
> >  should keep its source in remote-testgit.bash and generate it,
> 
> Why generate it? There's nothing to generate. python's source code
> needs regeneration, bash's code doesn't.

We fix up the #!-lines on all of the existing shell scripts (as well as
python and perl). Wouldn't we want to do the same for people who have
bash in an alternate location?

As the series is now, people with bash in their PATH, but not in
/bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
git-remote-testgit hardcodes /bin/bash).

-Peff

^ permalink raw reply

* Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: W. Trevor King @ 2012-11-28  2:42 UTC (permalink / raw)
  To: Heiko Voigt
  Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121127232858.GA4742@book.hvoigt.net>

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

On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > The v4 series leaves the remote branch amigious, but it helps you
> > point the local branch at the right hash so that future calls to
> > 
> >   $ git submodule foreach 'git pull'
> > 
> > can use the branch's .git/modules/<name>/config settings.
> 
> But IMO thats the functionality which should be implemented in submodule
> update and not left to the user.

Then you might need submodule.<name>.local-branch,
submodule.<name>.remote-repository, and submodule.<name>.remote-branch
to configure

  $ git checkout submodule.<name>.local-branch
  $ git pull submodule.<name>.remote-repository submodule.<name>.remote-branch

and this would ignore the $sha1 stored in the gitlink (which all of
the other update commands use).  This ignoring-the-$sha1 bit made me
think that a built-in pull wasn't a good fit for 'submodule update'.
Maybe if it went into a new 'submodule pull'?  Then users have a clear
distinction:

* 'update' to push superproject $sha1 changes into the submodules
* 'pull' to push upstream-branch changes into the submodules

> > > I would think more of some convention like:
> > > 
> > > 	$ git checkout -t origin/$branch
> > > 
> > > when first initialising the submodule with e.g.
> > > 
> > > 	$ git submodule update --init --branch
> > > 
> > > Then later calls of
> > > 
> > > 	$ git submodule update --branch
> > > 
> > > would have a branch configured to pull from. I imagine that results in
> > > a similar behavior gerrit is doing on the server side?
> > 
> > That sounds like it's doing pretty much the same thing.  Can you think
> > of a test that would distinguish it from my current v4 implementation?
> 
> Well the main difference is that gerrit is automatically updating the
> superproject AFAIK. I would like it if we could implement the same
> workflow support in the submodule script. It seems to me that this is
> already proven to be useful workflow.

Ah, sorry, I meant the configuring which remote branch you were
pulling from happens at submodule initialization (via .git/modules/…)
for both your workflow and my v4.

You're right that having a builtin pull is different from my v4.

> https://github.com/hvoigt/git/commits/hv/floating_submodules_draft

I looked over this before, but maybe not thoroughly enough ;).

> > > How about reusing the -b|--branch option for add? Since we only change
> > > the behavior when submodule.$name.update is set to branch it seems
> > > reasonable to me. Opinions?
> > 
> > That was the approach I used in v1, but people were concerned that we
> > would be stomping on previously unclaimed config space.  Since noone
> > has pointed out other uses besides Gerrit's very similar case, I'm not
> > sure if that is still an issue.
> 
> Could you point me to that mail? I cannot seem to find it in my archive.

Hmm.  It seems like Phil's initial response was (accidentally?) off
list.  The relevant portion was:

On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
> Some projects now use the 'branch' config value to record the tracking
> branch for the submodule.  Some ascribe different meaning to the
> configuration if the value is given vs. undefined.  For example, see
> the Gerrit submodule-subscription mechanism.  This change will cause
> those workflows to behave differently than they do now.
>
> I do like the idea, but I wish it had a different name for the
> recording.  Maybe --record-branch=${BRANCH} as an extra switch so the
> action is explicitly requested.

As I said, I'm happy to go back to --branch if opinions have changed.

On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > > > Because you need to recurse through submodules for `update --branch`
> > > > even if "$subsha1" == "$sha1", I had to amend the conditional
> > > > controlling that block.  This broke one of the existing tests, which I
> > > > "fixed" in patch 4.  I think a proper fix would involve rewriting
> > > > 
> > > >   (clear_local_git_env; cd "$sm_path" &&
> > > >    ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> > > >     test -z "$rev") || git-fetch)) ||
> > > >   die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > > > 
> > > > but I'm not familiar enough with rev-list to want to dig into that
> > > > yet.  If feedback for the earlier three patches is positive, I'll work
> > > > up a clean fix and resubmit.
> > > 
> > > You probably need to separate your handling here. The comparison of the
> > > currently checked out sha1 and the recorded sha1 is an optimization
> > > which skips unnecessary fetching in case the submodules commits are
> > > already correct. This code snippet checks whether the to be checked out
> > > sha1 is already local and also skips the fetch if it is. We should not
> > > break that.
> > 
> > Agreed.  However, determining if the target $sha1 is local should have
> > nothing to do with the current checked out $subsha1.
> 
> See my draft or the diff below for an illustration of the splitup.
> 
> [snip diff]

This looks fine, but my current --branch implementation (which doesn't
pull) is only a thin branch-checkout layer on top of the standard
`update` functionality.  I'm still unsure if built-in pulls are worth
the configuration trouble.  I'll sleep on it.  Maybe I'll feel better
about them tomorrow ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/7] New remote-bzr remote helper
From: Junio C Hamano @ 2012-11-28  2:37 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <CAMP44s3t54zYFha_qsDrg0JDZ52q8=WTs7q0rJ9qZL8kVCVWKA@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> At this point, both have been cooking for a week or more in 'next',
>> there is no existing users, they are on the fringe so breakages in
>> them won't negatively affect anybody anyway.  So it doesn't matter
>> much if they are merged to 'master' and then fixed up with follow up
>> patches after that, or fixed up with follow up patches while they
>> are in 'next', as they won't be rewound and restarted from scratch.
>
> The fixes are affecting some people, that's why I did them. Some were
> reported here in the mailing list, and some in my github's clone:
>
> https://github.com/felipec/git/issues?page=1&state=closed

Are you talking about -hg or -bzr or both?

In any case, I am mostly concerned about *my* next release, whose
rc0 will be tagged sometime this week or the next week.

People who have been bitten by bugs from *your* tree or versions in
'next' do not count.  When I said "no existing users", I was talking
about the end users who need rock solid stable "releases" because
tagged versions are the only ones they use.

If the code of these topics is still in flux and needs constant
fixes, probably it is a better idea to cook them longer in 'next',
skipping the upcoming 1.8.1 release.  If we are going to go that
route, we can drop the v2 fc/remote-bzr and queue v3 when we rewind
the tip of 'next' after 1.8.1 release (by that time you may have v4
of the series, but then we can skip v3).  Is that more preferrable
than rushing these topics forward before they are ready for general
audience?

^ permalink raw reply

* Re: [PATCH 0/4] remote-helpers: general fixes
From: Junio C Hamano @ 2012-11-28  2:30 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <7vfw3ujxh8.fsf@alter.siamese.dyndns.org>

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

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> These are general fixes, some for old versions of bazaar, mercurial, and
>> python. Some of these have already been sent, but here they go alone so they
>> are not missed.
>>
>> The bazaar fixes are on top of the series v3 which is still not in 'pu'.
>
> Please stop then.  Its v2 has been cooking in 'next' and it won't be
> replaced.

Picked up the -hg bit.  The other two has to wait until
fc/remote-bzr gets updated (but see other thread).

^ permalink raw reply

* [PATCH] fsck: warn about '.' and '..' in trees
From: Jeff King @ 2012-11-28  2:27 UTC (permalink / raw)
  To: git

A tree with meta-paths like '.' or '..' does not work well
with git; the index will refuse to load it or check it out
to the filesystem (and even if we did not have that safety,
it would look like we were overwriting an untracked
directory). For the same reason, it is difficult to create
such a tree with regular git.

Let's warn about these dubious entries during fsck, just in
case somebody has created a bogus tree (and this also lets
us prevent them from propagating when transfer.fsckObjects
is set).

Signed-off-by: Jeff King <peff@peff.net>
---
I don't think this is happening in the wild, but I did see somebody
playing around with libgit2 make such a tree (and it is easy to do with
git-mktree, of course).

Technically one could use git with such a tree as long as you never ever
checked out the result, but I think it is sufficiently crazy that we
should probably detect it, just in case.

Should we also detect ".git" in the same way? It is similarly treated
specially by verify_path(), and its presence is likely to cause weird
errors.

 fsck.c          | 10 ++++++++++
 t/t1450-fsck.sh | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/fsck.c b/fsck.c
index 7395ef6..31c9a51 100644
--- a/fsck.c
+++ b/fsck.c
@@ -142,6 +142,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 	int has_null_sha1 = 0;
 	int has_full_path = 0;
 	int has_empty_name = 0;
+	int has_dot = 0;
+	int has_dotdot = 0;
 	int has_zero_pad = 0;
 	int has_bad_modes = 0;
 	int has_dup_entries = 0;
@@ -168,6 +170,10 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 			has_full_path = 1;
 		if (!*name)
 			has_empty_name = 1;
+		if (!strcmp(name, "."))
+			has_dot = 1;
+		if (!strcmp(name, ".."))
+			has_dotdot = 1;
 		has_zero_pad |= *(char *)desc.buffer == '0';
 		update_tree_entry(&desc);
 
@@ -217,6 +223,10 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
 		retval += error_func(&item->object, FSCK_WARN, "contains full pathnames");
 	if (has_empty_name)
 		retval += error_func(&item->object, FSCK_WARN, "contains empty pathname");
+	if (has_dot)
+		retval += error_func(&item->object, FSCK_WARN, "contains '.'");
+	if (has_dotdot)
+		retval += error_func(&item->object, FSCK_WARN, "contains '..'");
 	if (has_zero_pad)
 		retval += error_func(&item->object, FSCK_WARN, "contains zero-padded file modes");
 	if (has_bad_modes)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 08aa24c..0b5c30b 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -237,4 +237,20 @@ test_expect_success 'fsck notices submodule entry pointing to null sha1' '
 	)
 '
 
+test_expect_success 'fsck notices "." and ".." in trees' '
+	(
+		git init dots &&
+		cd dots &&
+		blob=$(echo foo | git hash-object -w --stdin) &&
+		tab=$(printf "\\t") &&
+		git mktree <<-EOF &&
+		100644 blob $blob$tab.
+		100644 blob $blob$tab..
+		EOF
+		git fsck 2>out &&
+		cat out &&
+		grep "warning.*\\." out
+	)
+'
+
 test_done
-- 
1.8.0.207.gdf2154c

^ permalink raw reply related

* Re: [PATCH 0/4] remote-helpers: general fixes
From: Felipe Contreras @ 2012-11-28  2:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw3ujxh8.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 3:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> These are general fixes, some for old versions of bazaar, mercurial, and
>> python. Some of these have already been sent, but here they go alone so they
>> are not missed.
>>
>> The bazaar fixes are on top of the series v3 which is still not in 'pu'.
>
> Please stop then.  Its v2 has been cooking in 'next' and it won't be
> replaced.

Cooking since when? 9 days ago? I sent the series 17 days ago. But
suit yourself. I re-rolled for a reason.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v3 0/7] New remote-bzr remote helper
From: Felipe Contreras @ 2012-11-28  2:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <7vsj7ujxr2.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 2:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> OK.  Both fc/remote-hg and fc/remote-bzr are slated for 'master'
>>> soonish, but I take the above to mean that fc/remote-hg is ready
>>> while it is better to wait for updates to fc/remote-bzr before
>>> merging it.
>>
>> I was waiting on both to be merged, let me see what I have pending and
>> would be nice to have before the merge.
>
> OK.
>
> At this point, both have been cooking for a week or more in 'next',
> there is no existing users, they are on the fringe so breakages in
> them won't negatively affect anybody anyway.  So it doesn't matter
> much if they are merged to 'master' and then fixed up with follow up
> patches after that, or fixed up with follow up patches while they
> are in 'next', as they won't be rewound and restarted from scratch.

The fixes are affecting some people, that's why I did them. Some were
reported here in the mailing list, and some in my github's clone:

https://github.com/felipec/git/issues?page=1&state=closed

I tried to minimize the changes in the last patch series I sent, I
have more, but those truly can wait.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v2 3/3] Add option to transpose parents of merge commit
From: Junio C Hamano @ 2012-11-28  2:18 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git, Aaron Schrab
In-Reply-To: <1354057217-65886-4-git-send-email-draenog@pld-linux.org>

Kacper Kornet <draenog@pld-linux.org> writes:

> +--transpose-parents::
> +	Transpose the parents in the final commit. The change is made
> +	just before the commit so the meaning of 'our' and 'their'
> +	concepts remains the same (i.e. 'our' means current branch before
> +	the merge).
> +

How does this interact with Octopus merges?

> diff --git a/builtin/commit.c b/builtin/commit.c
> index ee0e884..ab2b844 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1477,6 +1477,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	} else if (whence == FROM_MERGE) {
>  		struct strbuf m = STRBUF_INIT;
>  		FILE *fp;
> +		int reversed_order=0;

Style. s/=/ = /;

> +	OPT_BOOLEAN(0, "transpose-parents", &reversed_order, N_("reverse order of parents")

It smells more like "--reverse-parents" (if you deal only with
two-head merges), no?

^ permalink raw reply

* Re: [PATCH v2 2/3] Allow for MERGE_MODE to specify more then one mode
From: Junio C Hamano @ 2012-11-28  2:17 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: git, Aaron Schrab
In-Reply-To: <1354057217-65886-3-git-send-email-draenog@pld-linux.org>

Kacper Kornet <draenog@pld-linux.org> writes:

> Presently only one merge mode exists: non-fast-forward. But in future
> the second one (transpose-parents) will be added, so the need to read
> all lines of MERGE_MODE.
>
> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> ---
>  builtin/commit.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 273332f..ee0e884 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1427,7 +1427,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	unsigned char sha1[20];
>  	struct ref_lock *ref_lock;
>  	struct commit_list *parents = NULL, **pptr = &parents;
> -	struct stat statbuf;
>  	int allow_fast_forward = 1;
>  	struct commit *current_head = NULL;
>  	struct commit_extra_header *extra = NULL;
> @@ -1481,11 +1480,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  
>  		if (!reflog_msg)
>  			reflog_msg = "commit (merge)";
> -		if (!stat(git_path("MERGE_MODE"), &statbuf)) {
> -			if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
> -				die_errno(_("could not read MERGE_MODE"));
> -			if (!strcmp(sb.buf, "no-ff"))
> -				allow_fast_forward = 0;
> +		if((fp = fopen(git_path("MERGE_MODE"), "r"))) {

Style: s/if((fp/if ((fp/;

> +			while (strbuf_getline(&m, fp, '\n') != EOF) {
> +				if (!strcmp(m.buf, "no-ff"))
> +					allow_fast_forward = 0;
> +			}
> +			fclose(fp);

This needs a bit more careful planning for interacting with other
people's programs, I suspect.

Your updated builtin/merge.c may write an extra LF after no-ff to
make this parser to grok it, but it is entirely plausible that
people have their own Porcelain that writes "no-ff" without LF
(because that is what we read from this file, and I suspect the
current code would ignore "no-ff\n").

At least strbuf_getline() would give us "no-ff" when either "no-ff"
or "no-ff\n" terminates the file, so updated code would be able to
grok what other people would write, but if other people want to read
MERGE_MODE we write, at least we shouldn't break them when we only
write no-ff in it (once you start writing "reverse-parents" in the
file, they will be broken anyway, as they do not currently expect
such token in this file).

I am starting to wonder if this is worth it, though...

^ permalink raw reply

* Re: [PATCH v6 p2 3/9] transport-helper: trivial code shuffle
From: Felipe Contreras @ 2012-11-28  2:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Ilari Liusvaara, Sverre Rabbelier, Elijah Newren,
	Thiago Farina
In-Reply-To: <7vwqx6jxv2.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 2:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> This is not just "just shuffle the die to make it explicit" but it
>>> does change the semantics; earlier ref->deletion was perfectly fine
>>> as long as data->refspecs is not given, but the new code always
>>> dies.
>>>
>>> If this semantic change is a good thing, please explain why it is so
>>> in the log message.  If the change is "it does not matter because
>>> when data->refspecs is not given and ref->deletion is set, we die
>>> later elsewhere in the code anyway", then it needs to be described.
>>
>> refspecs are optional, but when they are not present the code doesn't
>> work at all. This patch changes the behavior that was totally broken
>> anyway.
>
> In case it was not clear, I did not request/expect responses in the
> discussion thread, but a rerolled series with updated description.

An updated description that is irrelevant; the stuff is totally
broken, that's my point. But I'm tired of explaining it, and showing
it with test cases, and patches that fix those test cases, it's not my
itch, and "the other camp" doesn't even bother to acknowledge that
indeed remote helpers without marks just don't work, or even utter a
single word replying to one of these patches, nothing.

Not blaming you, just saying that I don't think anybody cares, clearly
nobody is exercising this code.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-28  2:09 UTC (permalink / raw)
  To: esr; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <20121125215635.GA6937@thyrsus.com>

On Sun, Nov 25, 2012 at 10:56 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> And gitk is an integral part of git. But if you have different
>> numbers, what are they?

> Please don't waste further time on quibbling.  We all know that gitk is
> an uncomfortable special case and that the project would be far better
> off, maintainability-wise, if it were successfully ported to one if these
> other languages.  Trying to catch me out by triumphantly pointing at gitk
> is...juvenile.

Another bit of information I just realized, 'man git' lists gitk as a
'Main porcelain command' as high level as any git command can get.

-- 
Felipe Contreras

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ 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