* Re: [PATCH v4 00/13] New remote-hg helper
From: Thomas Adam @ 2012-11-02 23:18 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jeff King, Michael J Gruber, Johannes Schindelin, git list,
Junio C Hamano, Sverre Rabbelier, Ilari Liusvaara,
Daniel Barkalow
In-Reply-To: <CAMP44s0N3k4b9SoKpkR=2-zSBb41tKW37tYhuxFfbooiLu59Kw@mail.gmail.com>
On 2 November 2012 18:39, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> I disagree. The open source process works not by making favors to each
> other, but by everyone sharing and improving the code, by
> *collaborating*. "I review your code if you review mine", or "if you
> by me a bear in the next conference" is not the spirit of open source,
> although it might happen often.
So shunning any attempt at explanation, and peddling your own thoughts
over and over again, irrespective of whether you contribute code or
not -- doesn't mean to say you're right, Felipe. And that's the
fundamental issue here -- your code speaks for itself, sure, no one
denies that, but the code is not even *half* of what makes up the
discussion. And so far, the surrounding context and attitude from you
doesn't help or enhance the process under which your code is reviewed.
And no, you cannot philosophise this, or wriggle out of it through
idealism or some other "charter" or "code of conduct" -- as reviewers
of your code, we have to interact with you to be able to better it.
But you seem very reluctant to do that.
The fact that we're even having the conversation is evident of that.
-- Thomas Adam
^ permalink raw reply
* Re: [PATCH] Enable parallelism in git submodule update.
From: Stefan Zager @ 2012-11-02 21:49 UTC (permalink / raw)
To: git; +Cc: Jens Lehmann, Heiko Voigt, Junio C Hamano
In-Reply-To: <CAHOQ7J-e=KBOsjoeTWsf1f+LNgaAxN974-FXNMeOy7B-FR0wyg@mail.gmail.com>
ping?
On Tue, Oct 30, 2012 at 11:11 AM, Stefan Zager <szager@google.com> wrote:
> This is a refresh of a conversation from a couple of months ago.
>
> I didn't try to implement all the desired features (e.g., smart logic
> for passing a -j parameter to recursive submodule invocations), but I
> did address the one issue that Junio insisted on: the code makes a
> best effort to detect whether xargs supports parallel execution on the
> host platform, and if it doesn't, then it prints a warning and falls
> back to serial execution.
>
> Stefan
>
> On Tue, Oct 30, 2012 at 11:03 AM, <szager@google.com> wrote:
>> The --jobs parameter may be used to set the degree of per-submodule
>> parallel execution.
>>
>> Signed-off-by: Stefan Zager <szager@google.com>
>> ---
>> Documentation/git-submodule.txt | 8 ++++++-
>> git-submodule.sh | 40 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index b4683bb..cb23ba7 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -14,7 +14,8 @@ SYNOPSIS
>> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>> 'git submodule' [--quiet] init [--] [<path>...]
>> 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
>> - [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>> + [--reference <repository>] [--merge] [--recursive]
>> + [-j|--jobs [jobs]] [--] [<path>...]
>> 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
>> [commit] [--] [<path>...]
>> 'git submodule' [--quiet] foreach [--recursive] <command>
>> @@ -146,6 +147,11 @@ If the submodule is not yet initialized, and you just want to use the
>> setting as stored in .gitmodules, you can automatically initialize the
>> submodule with the `--init` option.
>> +
>> +By default, each submodule is treated serially. You may specify a degree of
>> +parallel execution with the --jobs flag. If a parameter is provided, it is
>> +the maximum number of jobs to run in parallel; without a parameter, all jobs are
>> +run in parallel.
>> ++
>> If `--recursive` is specified, this command will recurse into the
>> registered submodules, and update any nested submodules within.
>> +
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index ab6b110..60a5f96 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
>> USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
>> or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>> or: $dashless [--quiet] init [--] [<path>...]
>> - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>> + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-j|--jobs [jobs]] [--] [<path>...]
>> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
>> or: $dashless [--quiet] foreach [--recursive] <command>
>> or: $dashless [--quiet] sync [--] [<path>...]"
>> @@ -500,6 +500,7 @@ cmd_update()
>> {
>> # parse $args after "submodule ... update".
>> orig_flags=
>> + jobs="1"
>> while test $# -ne 0
>> do
>> case "$1" in
>> @@ -518,6 +519,20 @@ cmd_update()
>> -r|--rebase)
>> update="rebase"
>> ;;
>> + -j|--jobs)
>> + case "$2" in
>> + ''|-*)
>> + jobs="0"
>> + ;;
>> + *)
>> + jobs="$2"
>> + shift
>> + ;;
>> + esac
>> + # Don't preserve this arg.
>> + shift
>> + continue
>> + ;;
>> --reference)
>> case "$2" in '') usage ;; esac
>> reference="--reference=$2"
>> @@ -551,11 +566,34 @@ cmd_update()
>> shift
>> done
>>
>> + # Correctly handle the case where '-q' came before 'update' on the command line.
>> + if test -n "$GIT_QUIET"
>> + then
>> + orig_flags="$orig_flags -q"
>> + fi
>> +
>> if test -n "$init"
>> then
>> cmd_init "--" "$@" || return
>> fi
>>
>> + if test "$jobs" != 1
>> + then
>> + if ( echo test | xargs -P "$jobs" true 2>/dev/null )
>> + then
>> + if ( echo test | xargs --max-lines=1 true 2>/dev/null ); then
>> + max_lines="--max-lines=1"
>> + else
>> + max_lines="-L 1"
>> + fi
>> + module_list "$@" | awk '{print $4}' |
>> + xargs $max_lines -P "$jobs" git submodule update $orig_flags
>> + return
>> + else
>> + echo "Warn: parallel execution is not supported on this platform."
>> + fi
>> + fi
>> +
>> cloned_modules=
>> module_list "$@" | {
>> err=
>> --
>> 1.7.7.3
>>
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2012, #09; Mon, 29)
From: Ramsay Jones @ 2012-11-02 21:33 UTC (permalink / raw)
To: Jeff King; +Cc: Chris Rorvick, git
In-Reply-To: <20121102095632.GA30221@sigill.intra.peff.net>
Jeff King wrote:
> On Fri, Nov 02, 2012 at 05:43:00AM -0400, Jeff King wrote:
>
>> Yeah, I think that is it. IIRC, Ramsay is on cygwin, and I noticed this
>> in perl 5.16's POSIX.xs:
>>
>> [...]
>> * (4) The CRT strftime() "%Z" implementation calls __tzset(). That
>> * calls CRT tzset(), but only the first time it is called, and in turn
>> * that uses CRT getenv("TZ") to retrieve the timezone info from the CRT
>> * local copy of the environment and hence gets the original setting as
>> * perl never updates the CRT copy when assigning to $ENV{TZ}.
>> [...]
>> I wonder if Ramsay has an older perl that does not do this special
>> hackery right. I'll see if I can dig up where it first appeared.
Hmm, sorry for not specifying this upfront, but this failure is on Linux. ;-)
(Linux is my main platform, but I like to keep cygwin working because it has
kept me sane on Windows ever since (about) 1995 ...)
"Stranger in a strange land" ;-)
This test is skipped on cygwin because I don't have cvs (or cvsps) installed
on cygwin. (I have cvs-1.11.1p1.tar.gz and cvsps-2.2b1.tar.gz lying around
on cygwin, so I could probably build them from source and do some testing if
you would like me too. Just let me know.)
This test is skipped on MinGW because cvsps is not installed. (I have just
tried building cvsps, but there are compilation errors ...).
I'm using perl v5.8.8 on Linux.
> It looks like that code went into perl 5.11.
>
> I wonder, even with this fix, though, if we need to be calling tzset to
> reliably update from the environment. It sounds like it _should_ happen
> automatically, except that if the CRT is calling the internal tzset, it
> would not do the perl "grab from $ENV" magic. Calling tzset would make
> sure the internal TZ is up to date.
>
> Ramsay, what happens with this patch on top?
This patch fixes the test for me.
Thanks!
ATB,
Ramsay Jones
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index ceb119d..4c44050 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -24,11 +24,12 @@ use File::Basename qw(basename dirname);
> use Time::Local;
> use IO::Socket;
> use IO::Pipe;
> -use POSIX qw(strftime dup2 ENOENT);
> +use POSIX qw(strftime tzset dup2 ENOENT);
> use IPC::Open2;
>
> $SIG{'PIPE'}="IGNORE";
> $ENV{'TZ'}="UTC";
> +tzset();
>
> our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r, $opt_R);
> my (%conv_author_name, %conv_author_email, %conv_author_tz);
> @@ -855,8 +856,10 @@ sub commit {
> }
>
> $ENV{'TZ'}=$author_tz;
> + tzset();
> my $commit_date = strftime("%s %z", localtime($date));
> $ENV{'TZ'}="UTC";
> + tzset();
> $ENV{GIT_AUTHOR_NAME} = $author_name;
> $ENV{GIT_AUTHOR_EMAIL} = $author_email;
> $ENV{GIT_AUTHOR_DATE} = $commit_date;
> .
>
^ permalink raw reply
* Re: [PATCH] gitweb.perl: fix %highlight_ext
From: rh @ 2012-11-02 21:18 UTC (permalink / raw)
To: git
In-Reply-To: <20121102145425.GC11170@sigill.intra.peff.net>
Peff and the Final Arbiter,
On Fri, 2 Nov 2012 10:54:25 -0400
Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 09:42:07AM -0700, rh wrote:
>
> > I also consolidated exts where applicable.
> > i.e. c and h maps to c
> >
> >
> > --
> >
> > diff --git a/a/gitweb.cgi b/b/gitweb.cgi
> > index 060db27..155b238 100755
> > --- a/a/gitweb.cgi
> > +++ b/b/gitweb.cgi
> > @@ -246,19 +246,19 @@ our %highlight_basename = (
> > 'Makefile' => 'make',
> > );
> > # match by extension
> > +
> > our %highlight_ext = (
> > # main extensions, defining name of syntax;
> > # see files in /usr/share/highlight/langDefs/ directory
> > - map { $_ => $_ }
> > - qw(py c cpp rb java css php sh pl js tex bib xml
> > awk bat ini spec tcl sql make),
> > + (map { $_ => $_ } qw(py rb java css js tex bib xml awk bat
> > ini spec tcl sql)),
> > # alternate extensions, see /etc/highlight/filetypes.conf
> > - 'h' => 'c',
> > - map { $_ => 'sh' } qw(bash zsh ksh),
> > - map { $_ => 'cpp' } qw(cxx c++ cc),
> > - map { $_ => 'php' } qw(php3 php4 php5 phps),
> > - map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
> > - map { $_ => 'make'} qw(mak mk),
> > - map { $_ => 'xml' } qw(xhtml html htm),
> > + (map { $_ => 'c' } qw(c h)),
> > + (map { $_ => 'sh' } qw(sh bash zsh ksh)),
> > + (map { $_ => 'cpp' } qw(cpp cxx c++ cc)),
> > + (map { $_ => 'php' } qw(php php3 php4 php5 phps)),
> > + (map { $_ => 'pl' } qw(pl perl pm)), # perhaps also 'cgi'
> > + (map { $_ => 'make'} qw(make mak mk)),
> > + (map { $_ => 'xml' } qw(xml xhtml html htm)),
>
> I think the patch itself looks OK, but:
>
> 1. It isn't formatted to apply with git-am. Please use
> git-format-patch.
git format-patch command wouldn't work for me. I can see that you
don't need more stuff to do but not knowing git I couldn't find the
correct incantation to do this part. A problem with the files not
being in a git repo I think. I'll spare you details.
>
> 2. The commit message does not explain the reason for the change.
>
> 3. It isn't signed-off.
These I've done and submitted the patch again. (even though format
is probably not going to work with git-am.
>
> The first two are things I can fix up (though it is inconvenient for
> me to do so), but the third is a show-stopper. Please look through
> Documentation/SubmittingPatches, especially the bit about the
> Developer's Certificate of Origin, and re-send.
>
> -Peff
Thanks for your patience and help.
^ permalink raw reply
* [PATCH] gitweb.perl: fix %highlight_ext mappings
From: rh @ 2012-11-02 21:12 UTC (permalink / raw)
To: git
The previous change created a dictionary of one-to-one elements when
the intent was to map mutliple related types to one main type.
e.g. bash, ksh, zsh, sh all map to sh since they share similar syntax
This makes the mapping as the original change intended.
Signed-off-by: rh <richard_hubbe11@lavabit.com>
diff --git a/gitweb.cgi.orig b/gitweb.cgi
index 060db27..155b238 100755
--- a/gitweb.cgi.orig
+++ b/gitweb.cgi
@@ -246,19 +246,19 @@ our %highlight_basename = (
'Makefile' => 'make',
);
# match by extension
+
our %highlight_ext = (
# main extensions, defining name of syntax;
# see files in /usr/share/highlight/langDefs/ directory
- map { $_ => $_ }
- qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),
+ (map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),
# alternate extensions, see /etc/highlight/filetypes.conf
- 'h' => 'c',
- map { $_ => 'sh' } qw(bash zsh ksh),
- map { $_ => 'cpp' } qw(cxx c++ cc),
- map { $_ => 'php' } qw(php3 php4 php5 phps),
- map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
- map { $_ => 'make'} qw(mak mk),
- map { $_ => 'xml' } qw(xhtml html htm),
+ (map { $_ => 'c' } qw(c h)),
+ (map { $_ => 'sh' } qw(sh bash zsh ksh)),
+ (map { $_ => 'cpp' } qw(cpp cxx c++ cc)),
+ (map { $_ => 'php' } qw(php php3 php4 php5 phps)),
+ (map { $_ => 'pl' } qw(pl perl pm)), # perhaps also 'cgi'
+ (map { $_ => 'make'} qw(make mak mk)),
+ (map { $_ => 'xml' } qw(xml xhtml html htm)),
);
# You define site-wide feature defaults here; override them with
^ permalink raw reply related
* Re: Set core.ignorecase globally
From: Torsten Bögershausen @ 2012-11-02 20:05 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: Kirill Likhodedov, git
In-Reply-To: <20121102191516.e8b7d448ff76b45990e14b1d@domain007.com>
Am 2012-11-02 16:15, schrieb Konstantin Khomoutov:
> On Fri, 2 Nov 2012 19:03:37 +0400
> Konstantin Khomoutov <flatworm@users.sourceforge.net> wrote:
>
>>> Currently, core.ignorecase is set to true on case insensitive system
>>> like Windows or Mac on `git init` and `git clone`, and this setting
>>> is local to the created/cloned repository.
>> [...]
>>> I suggest to set this globally by default when Git is installed,
>>> because there is little sense to have this option false on case
>>> insensitive systems (it will lead to confusions when renaming a file
>>> by changing only the case of letters).
>>
>> Case sensitivity is a property of a file system, not the OS.
>> What if I mount a device with ext3 file system via ext2fsd driver in
>> on my Windows workstation? extN have POSIX semantics so it's
>> pointless to enforce case insensitivity on them. The same possibly
>> applies to NFS mounts.
>>
>> Also note that NTFS (at least by default) is case insensitive but is
>> case preserving, observe:
> [...]
>
> On the other hand, on NTFS, if I unset core.ignorecase or set it to
> false locally, `git mv foo Foo` fails to rename a tracked file "foo"
> with the "destination file exists" error. I would say I would expect it
> to work under the conditions I've just described. Not sure if this
> thould be considered a bug in Git for Windows or not -- would be great
> to hear opinions of the msysgit port developers.
I once made a patch for git and we concluded that is is not worth
to put that into main git because you always can do:
git mv foo tmp && git mv tmp Foo
or
git mv -f foo Foo
(But use the -f option with care)
/Torsten
^ permalink raw reply
* Re: Wrap commit messages on `git commit -m`
From: Ramkumar Ramachandra @ 2012-11-02 19:37 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Kevin, Lars Gullik Bjønnes, git
In-Reply-To: <20121101221203.GE6213@elie.Belkin>
Jonathan Nieder wrote:
> Kevin wrote:
>
>> As I see it, the problem is not the possibility to add new lines, but
>> colleagues being too lazy to add them.
>
> I suspect the underlying problem is that we make it too hard to tell
> git which text editor to run.
Don't we just use $EDITOR?
> Ram, what platform do your colleagues use?
Red Hat Enterprise Linux 5.
Ram
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-11-02 19:20 UTC (permalink / raw)
To: Jeff King
Cc: Michael J Gruber, Johannes Schindelin, git, Junio C Hamano,
Sverre Rabbelier, Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <CAMP44s0N3k4b9SoKpkR=2-zSBb41tKW37tYhuxFfbooiLu59Kw@mail.gmail.com>
On Fri, Nov 2, 2012 at 7:39 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> As a rule, I don't see much value in writing a framework that works
> only for one case, that smells more like over-engineering. If we had
> two cases (hg and bzr), then we might be able to know with a modicum
> of certainty what such a framework should have. So I would prefer to
> have two standalone remote-helpers, and _then_ do a framework to
> simplify both, but not before. But that's my personal opinion.
>
> Now that I have free time, I might be able to spend time writing such
> a proof-of-concept remote-bzr, and a simple framework. But I would be
> concentrated on remote-hg.
Actually, there's no point in that; there's already a git-remote-bzr:
http://bazaar.launchpad.net/~bzr-git/bzr-git/trunk/view/head:/git-remote-bzr
So, what do we need a python framework for?
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-11-02 18:39 UTC (permalink / raw)
To: Jeff King
Cc: Michael J Gruber, Johannes Schindelin, git, Junio C Hamano,
Sverre Rabbelier, Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <20121102144618.GA11170@sigill.intra.peff.net>
On Fri, Nov 2, 2012 at 3:46 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Oct 31, 2012 at 05:11:39PM +0100, Felipe Contreras wrote:
>
>> > As a patch
>> > submitter, you ("generic you") want the attention of others as
>> > reviewers. It's in your own (again "generic you") interest not to put
>> > them off, in the same way as it's up to the submitter to argue why a
>> > patch is desirable and correct.
>>
>> Ah, so you are making me a favor by reviewing the code?
>
> I do not want to get embroiled in a discussion of manners and netiquette
> (or, for that matter, nazis). But I think this point is worth calling
> attention to, because it seems to be at the crux of the matter.
>
> Basically, my opinion is that yes, he is doing a favor to you by
> reviewing the code. Just as you have done us a favor by submitting the
> code.
Who cares about _us_? What is important is the bigger picture, git,
the project, and its users.
Yes, some people might think only about themselves, and that's their
choice, but I don't think us (the git project) should worry about
making me favors, only about improving the project.
> And this is not specific to this topic or to you as a submitter.
> It is a part of how the open source process works.
I disagree. The open source process works not by making favors to each
other, but by everyone sharing and improving the code, by
*collaborating*. "I review your code if you review mine", or "if you
by me a bear in the next conference" is not the spirit of open source,
although it might happen often.
> We have an existing code base that works well. It certainly has some
> bugs, and it certainly is missing some features. But people use it every
> day and are happy. The maintainers of that code base would want it to
> improve over time, but they would also have to be careful not to
> introduce regressions. And not just specific regressions in behavior; I
> mean regressions in overall quality. A half-implemented feature that
> crashes is worse than no feature at all. A change that fixes one bug but
> hurts the readability of the code, leading to many future bugs, is a net
> negative.
Indeed, but that has nothing to do with _us_ making _us_ favors, this
is about the *project*.
And having said that, this particular remote-hg helper is meant for
the *contrib* area, it's not half-implemented at all, and it's
certainly not crashing, or at least nobody has shown any evidence of
that. And for that matter, I'm sure I can point out to code that sits
in contrib that does meat that criteria (it's half-implemented, and
might even crash).
I know that you didn't mean otherwise, but in the context of
remote-hg, this seems hardly relevant.
And even if this wasn't for contrib, and this was half-implemented,
and this crashed... he still wouldn't be making _me_ any favors. The
review is for the project, not for me.
> So when a contributor shows up with code, we are very grateful that
> they've spent their time improving our software. But at the same time,
> we must recognize that the contributor is generally scratching their own
> itch. And we must make sure that in doing so, they did not hurt other
> people's use cases, nor regress the overall quality of the code base.
I fail to see how any code sitting in 'contrib/remote-hg' could do any of that.
> It is the job of the maintainer to measure the risk and reward of each
> change and to find a balance in accepting patches. But it's difficult to
> do alone, and that is why volunteer reviewers on the list are very
> valuable. They distribute the reviewing load across many brains, and in
> many cases have expertise in particular areas that the maintainer can
> rely on.
Yes, that is helpful, _for the project_.
> A submitter has scratched their own itch by writing the code. But if
> they cannot cooperate with reviewers enough to get feedback, then the
> maintainer has only two choices: review the patches themselves, or
> reject the change. And when there is conflict with the regular reviewers
> and the submitter, it is a red flag to the maintainer that it might not
> be worth spending a lot of time there.
That would be unfortunate, _for the project_, not for the submitter. I
can still run the code, and I can still share it on github.
Reviewers wouldn't be making _me_ any favors. It's the project that
benefits, and it's for the project that they should do it, or for
themselves, not for me.
> Does the code base suffer for this in the end? Perhaps. There are
> features we might reject that could have benefited everybody. But we
> might also be saving ourselves from the headaches caused by poorly
> thought-out changes. The system cannot work if everybody does not show
> up and cooperate.
Maybe, but most probably not. Take a look at this example:
% git log --oneline contrib/hg-to-git
44211e8 Correct references to /usr/bin/python which does not exist on FreeBSD
0def5b6 hg-to-git: fix COMMITTER type-o
b0c051d hg-to-git: don't import the unused popen2 module
af9a01e hg-to-git: use git init instead of git init-db
96f2395 hg-to-git: rewrite "git-frotz" to "git frotz"
2553ede hg-to-git: abort if the project directory is not a hg repo
6376cff hg-to-git: avoid raising a string exception
37a12dd hg-to-git: add --verbose option
13bf1a9 hg-to-git: fix parent analysis
1bc7c13 hg-to-git: improve popen calls
90e0653 hg-to-git: handle an empty dir in hg.
7c0d741 hg-to-git speedup through selectable repack intervals
98d47d4 Add hg-to-git conversion utility.
Is this fully implemented? Is this not crashing? Is the people
involved showing up and cooperating?
It doesn't look like that, yet, it has not given you any headaches,
probably because it's segregated in the contrib area, which what I
have tried to do with my patches. _Precisely_ to minimize possible
headaches.
Perhaps this is a case of double standards.
> Now, as for this specific topic: it is proposed for contrib, which means
> that expectations are lower, and the rest of git does not suffer too
> much if it has rough edges. At the same time, it also means that it
> could live fairly easily outside of the tree. In fact, I think Michael
> and others have been reasonably happy with their own out-of-tree
> implementation.
It certainly could... to the detriment of the project.
> I do think the proliferation of various implementations has made it hard
> for users to see which ones are worth trying. So I think there is value
> in carrying something in contrib/, as it would focus the attention of
> users, and of other developers to make improvements.
>
> So I think what I'd like to do is take your latest series into pu, with
> the intention of merging it into next soon, and then cooking it in next
> for a while. That would hopefully make it very easy for people following
> 'next' to try it out and see how it compares in the field with other
> tools they have used (the msysgit one, or others).
Excellent! I do agree that this would make it easier for everybody:
users to try it, developers to contribute, and keep track of the
changes, etc.
> I'm a little worried about hurting progress on the msysgit version; it
> sounds like the functionality of your implementation is at parity with
> that one (thanks to both you and Johannes for answering my other email
> asking for a summary). Johannes did mention that the design of their
> tool was meant to eventually facilitate more backends. That's something
> that might be valuable; on the other hand, that development hasn't been
> happening, and there has been no effort lately on getting it merged into
> git.git. I don't want to hold working code hostage to a future plan that
> might or might not happen. So I hope by keeping it in next for a bit,
> that will give msysgit people time to check it out and mobilize their
> efforts to improve their version if they would like.
Fair enough. I do think my version would facilitate more backends
because the code is very simple and it should be easy to see what you
need to change, and you don't need to get familiarized with any
framework, or classes of classes, and so on. I also think that if
needed, I could come up with such a framework as well, and the
resulting framework would be much simpler.
As a rule, I don't see much value in writing a framework that works
only for one case, that smells more like over-engineering. If we had
two cases (hg and bzr), then we might be able to know with a modicum
of certainty what such a framework should have. So I would prefer to
have two standalone remote-helpers, and _then_ do a framework to
simplify both, but not before. But that's my personal opinion.
Now that I have free time, I might be able to spend time writing such
a proof-of-concept remote-bzr, and a simple framework. But I would be
concentrated on remote-hg.
That would be the last one of the supposed advantages of the msysGit
remote-hg approach, and I make emphasis on supposed because we don't
know _for sure_ if the current framework would be useful for a
remote-bzr or not, *not* because I'm trying to offend anybody (in case
anybody is thinking that). Hopefully that would show the people that
have been working on this other tool that there is indeed value in
this code, put aside their personal differences and work together _for
the project_, but I wouldn't be holding my breath.
But to me _first_ what is important is to provide the functionality to
users, and _then_ is how we organize the code to make life easier for
us (the project).
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-11-02 18:01 UTC (permalink / raw)
To: Jeff King
Cc: Michael J Gruber, Johannes Schindelin, git, Junio C Hamano,
Sverre Rabbelier, Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <CAMP44s1P5Y_H24=ZKS5n_rUORf1dTiqg3qXm3bHcOiQ8K12PUQ@mail.gmail.com>
On Fri, Nov 2, 2012 at 5:41 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Nov 2, 2012 at 3:48 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Nov 01, 2012 at 05:08:52AM +0100, Felipe Contreras wrote:
>>
>>> > Turns out msysgit's remote-hg is not exporting the whole repository,
>>> > that's why it's faster =/
>>>
>>> It seems the reason is that it would only export to the point where
>>> the branch is checked out. After updating the to the tip I noticed
>>> there was a performance difference.
>>>
>>> I investigated and found two reasons:
>>>
>>> 1) msysgit's version doesn't export files twice, I've now implemented the same
>>> 2) msysgit's version uses a very simple algorithm to find out file changes
>>>
>>> This second point causes msysgit to miss some file changes. Using the
>>> same algorithm I get the same performance, but the output is not
>>> correct.
>>
>> Do you have a test case that demonstrates this? It would be helpful for
>> reviewers, but also helpful to msysgit people if they want to fix their
>> implementation.
>
> Cloning the mercurial repo:
>
> % hg log --stat -r 131
> changeset: 131:c9d51742471c
> parent: 127:44538462d3c8
> user: jake@edge2.net
> date: Sat May 21 11:35:26 2005 -0700
> summary: moving hgweb to mercurial subdir
>
> hgweb.py | 377
> ------------------------------------------------------------------------------------------
> mercurial/hgweb.py | 377
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 377 insertions(+), 377 deletions(-)
>
> % git show --stat 1f9bcfe7cc3d7af7b4533895181acd316ce172d8
> commit 1f9bcfe7cc3d7af7b4533895181acd316ce172d8
> Author: jake@edge2.net <none@none>
> Date: Sat May 21 11:35:26 2005 -0700
>
> moving hgweb to mercurial subdir
>
> mercurial/hgweb.py | 377
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 377 insertions(+)
I talked with some people in #mercurial, and apparently there is a
concept of a 'changelog' that is supposed to store these changes, but
since the format has changed, the content of it is unreliable. That's
not a big problem because it's used mostly for reporting purposes
(log, query), not for doing anything reliable.
To reliably see the changes, one has to compare the 'manifest' of the
revisions involved, which contain *all* the files in them.
That's what I was doing already, but I found a more efficient way to
do it. msysGit is using the changelog, which is quite fast, but not
reliable.
Unfortunately while going trough mercurial's code, I found an issue,
and it turns out that 1) is not correct.
In mercurial, a file hash contains also the parent file nodes, which
means that even if two files have the same content, they would not
have the same hash, so there's no point in keeping track of them to
avoid extracting the data unnecessarily, because in order to make sure
they are different, you need to extract the data anyway, defeating the
purpose.
Which means mercurial doesn't really behave as one would expect:
# add files with the same content
$ echo a > a
$ hg ci -Am adda
adding a
$ echo a >> a
$ hg ci -m changea
$ echo a > a
$ hg st --rev 0
$ hg ci -m reverta
$ hg log -G --template '{rev} {desc}\n'
@ 2 reverta
|
o 1 changea
|
o 0 adda
# check the difference between the first and the last revision
$ hg st --rev 0:2
M a
$ hg cat -r 0 a
a
$ hg cat -r 2 a
a
I will be checking again from where did I get the performance
improvements, but most likely it's from my implementation of
mercurial's repo.status().
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] gitweb.perl: fix %highlight_ext
From: rh @ 2012-11-02 17:41 UTC (permalink / raw)
To: git
In-Reply-To: <20121102145425.GC11170@sigill.intra.peff.net>
On Fri, 2 Nov 2012 10:54:25 -0400
Jeff King <peff@peff.net> wrote:
> On Mon, Oct 29, 2012 at 09:42:07AM -0700, rh wrote:
>
> > I also consolidated exts where applicable.
> > i.e. c and h maps to c
> >
> >
> > --
> >
> > diff --git a/a/gitweb.cgi b/b/gitweb.cgi
> > index 060db27..155b238 100755
> > --- a/a/gitweb.cgi
> > +++ b/b/gitweb.cgi
> > @@ -246,19 +246,19 @@ our %highlight_basename = (
> > 'Makefile' => 'make',
> > );
> > # match by extension
> > +
> > our %highlight_ext = (
> > # main extensions, defining name of syntax;
> > # see files in /usr/share/highlight/langDefs/ directory
> > - map { $_ => $_ }
> > - qw(py c cpp rb java css php sh pl js tex bib xml
> > awk bat ini spec tcl sql make),
> > + (map { $_ => $_ } qw(py rb java css js tex bib xml awk bat
> > ini spec tcl sql)),
> > # alternate extensions, see /etc/highlight/filetypes.conf
> > - 'h' => 'c',
> > - map { $_ => 'sh' } qw(bash zsh ksh),
> > - map { $_ => 'cpp' } qw(cxx c++ cc),
> > - map { $_ => 'php' } qw(php3 php4 php5 phps),
> > - map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
> > - map { $_ => 'make'} qw(mak mk),
> > - map { $_ => 'xml' } qw(xhtml html htm),
> > + (map { $_ => 'c' } qw(c h)),
> > + (map { $_ => 'sh' } qw(sh bash zsh ksh)),
> > + (map { $_ => 'cpp' } qw(cpp cxx c++ cc)),
> > + (map { $_ => 'php' } qw(php php3 php4 php5 phps)),
> > + (map { $_ => 'pl' } qw(pl perl pm)), # perhaps also 'cgi'
> > + (map { $_ => 'make'} qw(make mak mk)),
> > + (map { $_ => 'xml' } qw(xml xhtml html htm)),
>
> I think the patch itself looks OK, but:
>
> 1. It isn't formatted to apply with git-am. Please use
> git-format-patch.
>
> 2. The commit message does not explain the reason for the change.
>
> 3. It isn't signed-off.
>
> The first two are things I can fix up (though it is inconvenient for
> me to do so), but the third is a show-stopper. Please look through
> Documentation/SubmittingPatches, especially the bit about the
> Developer's Certificate of Origin, and re-send.
>
> -Peff
Thanks for taking the time to explain the workflow. If I can figure all this
out I will try to do what's needed. This is feeling like a strange cult
that I've bumped into!
--
"...she's got smarts and never farts and owns a chain of liquor stores."
^ permalink raw reply
* [PATCH] Add rm to submodule command.
From: Alex Linden Levy @ 2012-11-02 17:26 UTC (permalink / raw)
To: gitster, git; +Cc: Alex Linden Levy
This change removes the config entries in .gitmodules and adds it.
---
git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..29d950f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,6 +6,7 @@
dashless=$(basename "$0" | sed -e 's/-/ /')
USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
+ or: $dashless [--quiet] rm [-b branch] [-f|--force] [--] [<path>]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
@@ -370,6 +371,65 @@ Use -f if you really want to add it." >&2
die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
}
+cmd_rm()
+{
+ # parse $args after "submodule ... rm".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -b | --branch)
+ case "$2" in '') usage ;; esac
+ branch=$2
+ shift
+ ;;
+ -f | --force)
+ force=$1
+ ;;
+ -q|--quiet)
+ GIT_QUIET=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ sm_path=$1
+
+ # normalize path:
+ # multiple //; leading ./; /./; /../; trailing /
+ sm_path=$(printf '%s/\n' "$sm_path" |
+ sed -e '
+ s|//*|/|g
+ s|^\(\./\)*||
+ s|/\./|/|g
+ :start
+ s|\([^/]*\)/\.\./||
+ tstart
+ s|/*$||
+ ')
+
+
+ #edit .gitmodules
+ git config -f .gitmodules --remove-section submodule."$sm_path" ||
+ die "$(eval_gettext "Failed to rm submodule '\$sm_path' section")"
+
+ #get rid of the submodule
+ git rm --cached $force "$sm_path" ||
+ die "$(eval_gettext "Failed to rm submodule '\$sm_path'")"
+
+ #now add the .gitmodules change
+ git add ${force} .gitmodules ||
+ die "$(eval_gettext "Failed to remove submodule '\$sm_path'")"
+}
#
# Execute an arbitrary command sequence in each checked out
# submodule
@@ -1076,7 +1136,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | update | status | summary | sync)
+ add | rm | foreach | init | update | status | summary | sync)
command=$1
;;
-q|--quiet)
--
1.8.0.1.g3039071.dirty
^ permalink raw reply related
* Re: [PATCH v4 09/14] remote-testgit: report success after an import
From: Jeff King @ 2012-11-02 17:19 UTC (permalink / raw)
To: Felipe Contreras
Cc: Stefano Lattarini, git, Junio C Hamano, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <CAMP44s2T7iOdn6mWXqcUw97d8_-=pR=7ZQ5t_ygPGOphXW5cQw@mail.gmail.com>
On Fri, Nov 02, 2012 at 05:19:33PM +0100, Felipe Contreras wrote:
> > In any case, I agree that having a clean, understandable code as a
> > starting point is better than having a more "portable" but trickier
> > one right away. If it will need converting to POSIX, that can be
> > done as a follow up (and as we've both noticed, this would be the
> > only point where such a conversion might be problematic -- the other
> > changes would be trivial, almost automatic).
>
> As things are the options are:
>
> 1) Remove this code and move to POSIX sh. People looking for reference
> might scratch their heads as to why 'git push' is not showing the
> update.
> 2) Keep this code and remain in bash.
>
> Until we have a:
>
> 3) Replace this code with a clean POSIX sh alternative
>
> I would rather vote for 2)
I'm fine with bash. The critical thing is that it not break people's
"make test" if they do not have bash (or do not have bash as /bin/bash).
-Peff
^ permalink raw reply
* Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-11-02 16:41 UTC (permalink / raw)
To: Jeff King
Cc: Michael J Gruber, Johannes Schindelin, git, Junio C Hamano,
Sverre Rabbelier, Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <20121102144827.GB11170@sigill.intra.peff.net>
On Fri, Nov 2, 2012 at 3:48 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 01, 2012 at 05:08:52AM +0100, Felipe Contreras wrote:
>
>> > Turns out msysgit's remote-hg is not exporting the whole repository,
>> > that's why it's faster =/
>>
>> It seems the reason is that it would only export to the point where
>> the branch is checked out. After updating the to the tip I noticed
>> there was a performance difference.
>>
>> I investigated and found two reasons:
>>
>> 1) msysgit's version doesn't export files twice, I've now implemented the same
>> 2) msysgit's version uses a very simple algorithm to find out file changes
>>
>> This second point causes msysgit to miss some file changes. Using the
>> same algorithm I get the same performance, but the output is not
>> correct.
>
> Do you have a test case that demonstrates this? It would be helpful for
> reviewers, but also helpful to msysgit people if they want to fix their
> implementation.
Cloning the mercurial repo:
% hg log --stat -r 131
changeset: 131:c9d51742471c
parent: 127:44538462d3c8
user: jake@edge2.net
date: Sat May 21 11:35:26 2005 -0700
summary: moving hgweb to mercurial subdir
hgweb.py | 377
------------------------------------------------------------------------------------------
mercurial/hgweb.py | 377
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 377 insertions(+), 377 deletions(-)
% git show --stat 1f9bcfe7cc3d7af7b4533895181acd316ce172d8
commit 1f9bcfe7cc3d7af7b4533895181acd316ce172d8
Author: jake@edge2.net <none@none>
Date: Sat May 21 11:35:26 2005 -0700
moving hgweb to mercurial subdir
mercurial/hgweb.py | 377
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 377 insertions(+)
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 09/14] remote-testgit: report success after an import
From: Stefano Lattarini @ 2012-11-02 16:23 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <CAMP44s2T7iOdn6mWXqcUw97d8_-=pR=7ZQ5t_ygPGOphXW5cQw@mail.gmail.com>
On 11/02/2012 05:19 PM, Felipe Contreras wrote:
>
> [SNIP]
>
> As things are the options are:
>
> 1) Remove this code and move to POSIX sh. People looking for reference
> might scratch their heads as to why 'git push' is not showing the
> update.
> 2) Keep this code and remain in bash.
>
> Until we have a:
>
> 3) Replace this code with a clean POSIX sh alternative
>
> I would rather vote for 2)
>
That's perfectly fine with me, now that you've explained the rationale
for requiring bash in the first place (maybe adding a comment to the
script or the commit messages that reports this rationale for choosing
to require bash might be worthwhile; but it's no big deal anyway).
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v4 09/14] remote-testgit: report success after an import
From: Felipe Contreras @ 2012-11-02 16:19 UTC (permalink / raw)
To: Stefano Lattarini
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <5093EFF4.5080308@gmail.com>
On Fri, Nov 2, 2012 at 5:08 PM, Stefano Lattarini
<stefano.lattarini@gmail.com> wrote:
> On 11/02/2012 04:46 PM, Felipe Contreras wrote:
>>
>> In the end I liked this approach much better.
>>
>> If you have a solution for this that works in POSIX shell, I'll be
>> glad to consider it, but for the moment, I think a simple, easy to
>> understand and maintain code is more important, and if it needs bash,
>> so be it.
>>
> If this is a deliberate decision, it's ok with me. I'm just a "casual"
> reviewer here, not an active contributor, so I'll gladly accept
> preferences and decisions of the "active crew", once it's clear that
> they are deliberate and not the result of mistakes or confusion.
>
> In any case, I agree that having a clean, understandable code as a
> starting point is better than having a more "portable" but trickier
> one right away. If it will need converting to POSIX, that can be
> done as a follow up (and as we've both noticed, this would be the
> only point where such a conversion might be problematic -- the other
> changes would be trivial, almost automatic).
As things are the options are:
1) Remove this code and move to POSIX sh. People looking for reference
might scratch their heads as to why 'git push' is not showing the
update.
2) Keep this code and remain in bash.
Until we have a:
3) Replace this code with a clean POSIX sh alternative
I would rather vote for 2)
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
From: Felipe Contreras @ 2012-11-02 16:16 UTC (permalink / raw)
To: Stefano Lattarini
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <5093EEB4.3040402@gmail.com>
On Fri, Nov 2, 2012 at 5:03 PM, Stefano Lattarini
<stefano.lattarini@gmail.com> wrote:
> On 11/02/2012 04:42 PM, Felipe Contreras wrote:
>> What happens when you call this with:
>>
>> ./script "alias with spaces"
>>
> '$alias' will correctly expand to "alias with spaces". Try out:
>
> $ sh -c 'alias=$1; echo "$alias"' dummy '1 2*3'
> 1 2*3
>
> This works consistently with every known shell (even non-POSIX
> relics like Solaris /bin/sh).
All right.
>> _If_ we want this as POSIX, yeah.
>>
> Why don't we? Why add an extra requirement for a test that
>
> 1. can be easily written in POSIX shell, and
> 2. tests a feature that doesn't require bash to work (unless
> I'm sorely mistaken, that is)?
>
> Honest question. But of course, if the Git active contributors
> deem the extra requirement (which is not an invasive one, given
> how often bash is installed even on non-Linux systems) acceptable
> in order to have the test case simpler and clearer, feel free to
> disregard all my observations in this thread.
Because the code will be more complicated. Most of the changes
required are relatively small, so it's not a big issue, but I would
like to see how you replace the code that uses associative arrays. I
don't know know of a clean, simple way to do it in POSIX. If you can
find one, I don't see any strong reason to use bash.
>>>> +while read line; do
>>>> + case "$line" in
>>>>
>>> Useless double quoting (my previous observation about Git coding
>>> guidelines applies here as well, of course).
>>
>> What if line has multiple spaces?
>>
> Still no problem, as in the case of the 'alias=$1' assignment before:
>
> $ sh -c 'case $1 in *x" "x*) echo ok;; *) exit 1;; esac' dummy 'x x'
> ok
All right.
>>>> + echo "feature import-marks=$gitmarks"
>>>> + echo "feature export-marks=$gitmarks"
>>>> + git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs | \
>>>>
>>> Better avoid the tricky {foo,bar} bashism:
>>>
>>> git fast-export --use-done-feature \
>>> --import-marks="$testgitmarks" \
>>> --export-marks="$testgitmarks" \
>>> $refs | \
>>
>> If that's what we want, yeah.
>>
> Honestly, I find my longer-and-more-explicit version clearer, even
> if you can assume bash for your script. But that's a matter of
> personal preference (sorry for not stating that right away), so
> feel free to ignore it if you decide to keep the bash requirement
> in the end.
And I prefer the other form. I fact, I would prefer if both tools
simply had a --marks option set both, and didn't require the file to
be created beforehand. I've _never_ seen a situation where separate
marks for import and export made sense. But that's off-topic.
If you find a way to replace the associative arrays code, I have no
trouble in switching this to the POSIX version.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Lack of netiquette, was Re: [PATCH v4 00/13] New remote-hg helper
From: Felipe Contreras @ 2012-11-02 16:09 UTC (permalink / raw)
To: Michael J Gruber
Cc: Andreas Ericsson, René Scharfe, Junio C Hamano,
Johannes Schindelin, Jonathan Nieder, Jeff King, git,
Sverre Rabbelier, Ilari Liusvaara, Daniel Barkalow
In-Reply-To: <5093A873.9090701@drmicha.warpmail.net>
On Fri, Nov 2, 2012 at 12:03 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Andreas Ericsson venit, vidit, dixit 02.11.2012 10:38:
>> On 11/01/2012 02:46 PM, René Scharfe wrote:
>>>
>>> Also, and I'm sure you didn't know that, "Jedem das Seine" (to each
>>> his own) was the slogan of the Buchenwald concentration camp. For
>>> that reason some (including me) hear the unspoken cynical
>>> half-sentence "and some people just have to be sent to the gas
>>> chamber" when someone uses this proverb.
>>>
>>
>> It goes further back than that.
>>
>> "Suum cuique pulchrum est" ("To each his own is a beautiful thing") is
>> a latin phrase said to be used frequently in the roman senate when
>> senators politely agreed to disagree and let a vote decide the outcome
>> rather than debating further.
>>
>> Please don't let the twisted views of whatever nazi idiot thought it
>> meant "you may have the wrong faith and therefore deserve to die, so you
>> shall" pollute it. The original meaning is both poetic and democratic,
>> and I firmly believe most people have the original meaning to the fore
>> of their mind when using it. After all, very few people knowingly quote
>> nazi concentration camp slogans.
>>
>
> In fact, many German terms and words are "forbidden area" since Nazi
> times, but I don't think this one carries the same connotation.
>
> But that is a side track.
>
> Collaboration (and code review is a form of collaboration) requires
> communication. The linked code of conduct pages describe quite well how
> to ensure a productive environment in which "everyone" feels comfortable
> communicating and collaborating.
Yes, but that's assuming we want "everyone" to feel comfortable
communicating and collaborating. I cite again the example of the Linux
kernel, where certainly not "everyone" feels that way. But somehow
they manage to be perhaps the most successful software project in
history. And I would argue even more: it's _because_ not everyone
feels comfortable, it's because ideas and code are criticized freely,
and because only the ones that do have merit stand. If you are able to
take criticism, and you are not emotionally and personally attacked to
your code and your ideas, you would thrive in this environment. If you
don't want your precious little baby code to fight against the big
guys, then you shouldn't send it out to the world.
Junio mentioned "technical merit", and I believe for that open and
_honest_ communication is more important than making "everyone" feel
comfortable.
And FWIW I don't feel comfortable expressing my opinion any more,
because even if I criticize ideas and code on a *technical* basis, I'm
assumed to be referencing Nazism and whatnot without any regards of
what my original intentions were, or what I actually said, and
definitely not assuming good faith. And when asked for clarification
of what exactly that I said was offensive, I get no clear answer.
The dangers of "everyone" following the same style of communication,
and making "everyone" feel comfortable, is that "everyone" ends up
being the same kind of people, and the ones that don't fit the
definition of "everyone" feel like outsiders, or outright leave the
project. And you end up with an homogeneous group of people incapable
of criticizing each other honestly (on a technical basis), whether
it's because of lack of a different perspective, or unwillingness to
speak openly, or difficulty in finding the right polite words. I've
seen many projects fall into this, and erode with time, since nothing
important actually happens, and real deep issues within the code or
the community get ignored.
Anyway, I've yet to find what was actually wrong in the words I said.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 09/14] remote-testgit: report success after an import
From: Stefano Lattarini @ 2012-11-02 16:08 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <CAMP44s2ZPbda7yJ9UtOhWMqaKp4TaAgoyeUWUSrt0vco7RK+Tw@mail.gmail.com>
On 11/02/2012 04:46 PM, Felipe Contreras wrote:
>
> In the end I liked this approach much better.
>
> If you have a solution for this that works in POSIX shell, I'll be
> glad to consider it, but for the moment, I think a simple, easy to
> understand and maintain code is more important, and if it needs bash,
> so be it.
>
If this is a deliberate decision, it's ok with me. I'm just a "casual"
reviewer here, not an active contributor, so I'll gladly accept
preferences and decisions of the "active crew", once it's clear that
they are deliberate and not the result of mistakes or confusion.
In any case, I agree that having a clean, understandable code as a
starting point is better than having a more "portable" but trickier
one right away. If it will need converting to POSIX, that can be
done as a follow up (and as we've both noticed, this would be the
only point where such a conversion might be problematic -- the other
changes would be trivial, almost automatic).
Regards,
Stefano
^ permalink raw reply
* Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
From: Stefano Lattarini @ 2012-11-02 16:03 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <CAMP44s0n_O-7j329WyTxsWgVbCV2mEAnPG-WQeWBFdmHdfOjnQ@mail.gmail.com>
On 11/02/2012 04:42 PM, Felipe Contreras wrote:
> On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini
> <stefano.lattarini@gmail.com> wrote:
>
>>> +#!/bin/bash
>>>
>> I think git can't assume the existence of bash unconditionally, neither
>> in its scripts, nor in its tests (the exception being the tests on
>> bash completion, of course). This script probably need to be re-written
>> to be a valid POSIX shell script.
>
> Well, this is a _reference_ script, and that is used only for testing
> purposes. The test itself can be like the bash completion tests, and
> simply be skipped.
>
> The reason I chose bash is because associative arrays, which you see
> in a later patch.
>
>> It almost is, anyway, apart from the nits below ...
>>
>>> +# Copyright (c) 2012 Felipe Contreras
>>> +
>>> +alias="$1"
>>>
>> Just FYI: the double quoting here (and in several variable assignments
>> below) is redundant. You can portably write it as:
>>
>> alias=$1
>>
>> and still be safe in the face of spaces and metacharacters in $1.
>> I'm not sure whether the Git coding guidelines suggest the use of
>> quoting in this situation though; if this is the case, feel free
>> to disregard my observation.
>
> What happens when you call this with:
>
> ./script "alias with spaces"
>
'$alias' will correctly expand to "alias with spaces". Try out:
$ sh -c 'alias=$1; echo "$alias"' dummy '1 2*3'
1 2*3
This works consistently with every known shell (even non-POSIX
relics like Solaris /bin/sh).
>>> +url="$2"
>>> +
>>> +# huh?
>>> +url="${url#file://}"
>>> +
>>> +dir="$GIT_DIR/testgit/$alias"
>>> +prefix="refs/testgit/$alias"
>>> +refspec="refs/heads/*:${prefix}/heads/*"
>>> +
>>> +gitmarks="$dir/git.marks"
>>> +testgitmarks="$dir/testgit.marks"
>>> +
>>> +export GIT_DIR="$url/.git"
>>> +
>> I believe this should be rewritten as:
>>
>> GIT_DIR="$url/.git"; export GIT_DIR
>>
>> in order to be portable to all the POSIX shells targeted by Git.
>
> _If_ we want this as POSIX, yeah.
>
Why don't we? Why add an extra requirement for a test that
1. can be easily written in POSIX shell, and
2. tests a feature that doesn't require bash to work (unless
I'm sorely mistaken, that is)?
Honest question. But of course, if the Git active contributors
deem the extra requirement (which is not an invasive one, given
how often bash is installed even on non-Linux systems) acceptable
in order to have the test case simpler and clearer, feel free to
disregard all my observations in this thread.
>>> +mkdir -p "$dir"
>>> +
>>> +test -e "$gitmarks" || echo -n > "$gitmarks"
>>> +test -e "$testgitmarks" || echo -n > "$testgitmarks"
>>> +
>> The '-n' option to echo is not portable. To create an empty
>> file, you can just use
>>
>> : > file
>>
>> or
>>
>> true > file
>
> All right, thanks.
>
>>> +while read line; do
>>> + case "$line" in
>>>
>> Useless double quoting (my previous observation about Git coding
>> guidelines applies here as well, of course).
>
> What if line has multiple spaces?
>
Still no problem, as in the case of the 'alias=$1' assignment before:
$ sh -c 'case $1 in *x" "x*) echo ok;; *) exit 1;; esac' dummy 'x x'
ok
> To me it makes sense to quote it.
>
Surely it doesn't cause any problem to "over-quote" in this case;
it's better than risking to under-quote in other. I just pointed
out that the quoting it's not really necessary, in case you weren't
aware of that.
>>> + echo "feature import-marks=$gitmarks"
>>> + echo "feature export-marks=$gitmarks"
>>> + git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs | \
>>>
>> Better avoid the tricky {foo,bar} bashism:
>>
>> git fast-export --use-done-feature \
>> --import-marks="$testgitmarks" \
>> --export-marks="$testgitmarks" \
>> $refs | \
>
> If that's what we want, yeah.
>
Honestly, I find my longer-and-more-explicit version clearer, even
if you can assume bash for your script. But that's a matter of
personal preference (sorry for not stating that right away), so
feel free to ignore it if you decide to keep the bash requirement
in the end.
Regards,
Stefano
^ permalink raw reply
* Re: [PATCH v4 09/14] remote-testgit: report success after an import
From: Felipe Contreras @ 2012-11-02 15:46 UTC (permalink / raw)
To: Stefano Lattarini
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <5093D193.3030108@gmail.com>
On Fri, Nov 2, 2012 at 2:58 PM, Stefano Lattarini
<stefano.lattarini@gmail.com> wrote:
> On 11/02/2012 03:02 AM, Felipe Contreras wrote:
>> Doesn't make a difference for the tests, but it does for the ones
>> seeking reference.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> git-remote-testgit | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/git-remote-testgit b/git-remote-testgit
>> index 6c348b0..4e8b356 100755
>> --- a/git-remote-testgit
>> +++ b/git-remote-testgit
>> @@ -59,7 +59,18 @@ while read line; do
>> sed -e "s#refs/heads/#${prefix}/heads/#g"
>> ;;
>> export)
>> + declare -A before after
>> +
> If you convert this script to be a valid POSIX shell script (as I've
> suggested in my reply to [PATCH v4 04/14]), you'll need to get rid of
> this bashism (and those below) as well.
Yeah, but I don't want to. I originally used transitory files, and
used esoteric options of diff to do the same (which were probably not
portable).
In the end I liked this approach much better.
If you have a solution for this that works in POSIX shell, I'll be
glad to consider it, but for the moment, I think a simple, easy to
understand and maintain code is more important, and if it needs bash,
so be it.
>> + eval $(git for-each-ref --format='before[%(refname)]=%(objectname)')
>> +
>> git fast-import --{import,export}-marks="$testgitmarks" --quiet
>> +
>> + eval $(git for-each-ref --format='after[%(refname)]=%(objectname)')
>> +
>> + for ref in "${!after[@]}"; do
>> + test "${before[$ref]}" == "${after[$ref]}" && continue
>> + echo "ok $ref"
>> + done
>> echo
>> ;;
>> '')
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
From: Felipe Contreras @ 2012-11-02 15:42 UTC (permalink / raw)
To: Stefano Lattarini
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Elijah Newren, Ilari Liusvaara, Sverre Rabbelier
In-Reply-To: <5093D0DD.3050801@gmail.com>
On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini
<stefano.lattarini@gmail.com> wrote:
>> +#!/bin/bash
>>
> I think git can't assume the existence of bash unconditionally, neither
> in its scripts, nor in its tests (the exception being the tests on
> bash completion, of course). This script probably need to be re-written
> to be a valid POSIX shell script.
Well, this is a _reference_ script, and that is used only for testing
purposes. The test itself can be like the bash completion tests, and
simply be skipped.
The reason I chose bash is because associative arrays, which you see
in a later patch.
> It almost is, anyway, apart from the nits below ...
>
>> +# Copyright (c) 2012 Felipe Contreras
>> +
>> +alias="$1"
>>
> Just FYI: the double quoting here (and in several variable assignments
> below) is redundant. You can portably write it as:
>
> alias=$1
>
> and still be safe in the face of spaces and metacharacters in $1.
> I'm not sure whether the Git coding guidelines suggest the use of
> quoting in this situation though; if this is the case, feel free
> to disregard my observation.
What happens when you call this with:
./script "alias with spaces"
>> +url="$2"
>> +
>> +# huh?
>> +url="${url#file://}"
>> +
>> +dir="$GIT_DIR/testgit/$alias"
>> +prefix="refs/testgit/$alias"
>> +refspec="refs/heads/*:${prefix}/heads/*"
>> +
>> +gitmarks="$dir/git.marks"
>> +testgitmarks="$dir/testgit.marks"
>> +
>> +export GIT_DIR="$url/.git"
>> +
> I believe this should be rewritten as:
>
> GIT_DIR="$url/.git"; export GIT_DIR
>
> in order to be portable to all the POSIX shells targeted by Git.
_If_ we want this as POSIX, yeah.
>> +mkdir -p "$dir"
>> +
>> +test -e "$gitmarks" || echo -n > "$gitmarks"
>> +test -e "$testgitmarks" || echo -n > "$testgitmarks"
>> +
> The '-n' option to echo is not portable. To create an empty
> file, you can just use
>
> : > file
>
> or
>
> true > file
All right, thanks.
>> +while read line; do
>> + case "$line" in
>>
> Useless double quoting (my previous observation about Git coding
> guidelines applies here as well, of course).
What if line has multiple spaces? To me it makes sense to quote it.
>> + echo "feature import-marks=$gitmarks"
>> + echo "feature export-marks=$gitmarks"
>> + git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs | \
>>
> Better avoid the tricky {foo,bar} bashism:
>
> git fast-export --use-done-feature \
> --import-marks="$testgitmarks" \
> --export-marks="$testgitmarks" \
> $refs | \
If that's what we want, yeah.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
From: Jeff King @ 2012-11-02 15:38 UTC (permalink / raw)
To: karsten.blees; +Cc: git, msysgit, pro-logic
In-Reply-To: <20121102152616.GD11170@sigill.intra.peff.net>
On Fri, Nov 02, 2012 at 11:26:16AM -0400, Jeff King wrote:
> Still, I don't think we need to worry about performance regressions,
> because people who don't have a setup suitable for it will not turn on
> core.preloadindex in the first place. And if they have it on, the more
> places we use it, probably the better.
BTW, your patch was badly damaged in transit (wrapped, and tabs
converted to spaces). I was able to fix it up, but please check your
mailer's settings.
-Peff
--
*** 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
* Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-11-02 15:35 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Jonathan Nieder, git, Junio C Hamano,
Sverre Rabbelier, Elijah Newren
In-Reply-To: <20121102151955.GA24622@sigill.intra.peff.net>
On Fri, Nov 2, 2012 at 4:19 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Nov 02, 2012 at 04:17:14PM +0100, Johannes Schindelin wrote:
>> May I just ask to include a summary of that rationale into the commit
>> message rather than relying on people having internet access and knowing
>> where to look? Adding the following to the commit message would be good
>> enough for me:
>>
>> Note that
>>
>> $ git branch foo master~1
>> $ git fast-export foo master~1..master
>>
>> still does not update the "foo" ref, but a partial fix is better
>> than no fix.
>
> Yes, I think that makes a lot of sense.
>
> Felipe, I notice that you sent out a big "fast-export improvements"
> series. Does that supersede this?
Yes. I noticed this patch fixes other tests.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
From: Felipe Contreras @ 2012-11-02 15:34 UTC (permalink / raw)
To: Jeff King
Cc: Jonathan Nieder, git, Junio C Hamano, Sverre Rabbelier,
Johannes Schindelin, Elijah Newren
In-Reply-To: <20121102131255.GB2598@sigill.intra.peff.net>
On Fri, Nov 2, 2012 at 2:12 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 30, 2012 at 05:37:21PM -0700, Jonathan Nieder wrote:
>
>> If the commit does not have the SHOWN or UNINTERESTING flag set but it
>> is going to get the UNINTERESTING flag set during the walk because of
>> a negative commit listed on the command line, this patch won't help.
>
> Right, so my understanding of the situation is that doing this:
>
> $ git branch foo master~1
> $ git fast-export foo master~1..master
>
> won't show "foo", which seems wrong to me. _But_ we currently get that
> wrong already, so Felipe's patches are not making anything worse, but
> are fixing some situations (namely when master~1 is not mentioned on the
> command-line, but rather in a marks file).
>
> Is that correct?
Yes, that's correct. But my patch ("make sure refs are updated
properly") does _not_ change in any shape or form what happens with
what you specify in the command line, only what happens with marks.
Cheers.
--
Felipe Contreras
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox