Re: Wrap commit messages on `git commit -m`
Ramkumar Ramachandra wrote: > Jonathan Nieder wrote: >> Ram, what platform do your colleagues use? > > Red Hat Enterprise Linux 5. Oh, ok. In that case I blame habit. I think the best option you have is to just complain to your colleagues about the long lines. Then they would get a chance to explore what the UI currently offers and to complain to this list (perhaps via you :)) about missing features that would make their work easier. To put it another way: I don't see yet how a hypothetical "git commit --wrap-lines -m 'long message'" would make life easier than just running "git commit" and entering the message using $EDITOR. There's probably a UI or documentation bug lurking somewhere, so thanks for thinking about these things. Regards, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On Sat, Nov 3, 2012 at 12:18 AM, Thomas Adam wrote: > On 2 November 2012 18:39, Felipe Contreras 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. Who is saying I'm right? Certainly not me. I have explained patches over and over, even to the point that people apparently get offended, and now you say I should explain more? I'm sorry, but no, you cannot have your cake and eat it at the same time. Either you want me to explain things, or not. > 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. I would say it's the other way around, it's the attitude of other people that believe they are entitled to their opinion not being shined in a critical fashion, while at the same time being very critical themselves. If you want to disagree, fine, but it's still the project that gets hurt, not me, reviewing code is still for the benefit of the project. > 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. That's right, for the benefit of the project. > But you seem very reluctant to do that. Reluctant to what? Interact? I've answered every single question, and then some. I've also implemented tests, and addressed every criticism of this patch series however rude that criticism was thrown. It's actually the other way around. There's a thread about netiquette, precisely to avoid certain kinds of discussion. Show me a *single* instance where I've ignored a review comment, or whatever you mean by being reluctant to interact. > The fact that we're even having the conversation is evident of that. The fact that the sun raised at east and set at the west was evident that it was rotating around the Earth, but that, like many other assumptions, was wrong. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On 2 November 2012 18:39, Felipe Contreras 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable parallelism in git submodule update.
ping?
On Tue, Oct 30, 2012 at 11:11 AM, Stefan Zager 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, wrote:
>> The --jobs parameter may be used to set the degree of per-submodule
>> parallel execution.
>>
>> Signed-off-by: Stefan Zager
>> ---
>> 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] [--] [...]
>> 'git submodule' [--quiet] init [--] [...]
>> 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
>> - [--reference ] [--merge] [--recursive] [--]
>> [...]
>> + [--reference ] [--merge] [--recursive]
>> + [-j|--jobs [jobs]] [--] [...]
>> 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit)
>> ]
>> [commit] [--] [...]
>> 'git submodule' [--quiet] foreach [--recursive]
>> @@ -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 ]
>> [--] []
>> or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]
>> or: $dashless [--quiet] init [--] [...]
>> - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force]
>> [--rebase] [--reference ] [--merge] [--recursive] [--]
>> [...]
>> + or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force]
>> [--rebase] [--reference ] [--merge] [--recursive] [-j|--jobs
>> [jobs]] [--] [...]
>> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ]
>> [commit] [--] [...]
>> or: $dashless [--quiet] foreach [--recursive]
>> or: $dashless [--quiet] sync [--] [...]"
>> @@ -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
>> +
Re: What's cooking in git.git (Oct 2012, #09; Mon, 29)
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;
> .
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Set core.ignorecase globally
Am 2012-11-02 16:15, schrieb Konstantin Khomoutov: On Fri, 2 Nov 2012 19:03:37 +0400 Konstantin Khomoutov 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wrap commit messages on `git commit -m`
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On Fri, Nov 2, 2012 at 7:39 PM, Felipe Contreras 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On Fri, Nov 2, 2012 at 3:46 PM, Jeff King 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:
Re: [PATCH v4 00/13] New remote-hg helper
On Fri, Nov 2, 2012 at 5:41 PM, Felipe Contreras
wrote:
> On Fri, Nov 2, 2012 at 3:48 PM, Jeff King 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:j...@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: j...@edge2.net
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add rm to submodule command.
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 ] [--]
[]
+ or: $dashless [--quiet] rm [-b branch] [-f|--force] [--] []
or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]
or: $dashless [--quiet] init [--] [...]
or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force]
[--rebase] [--reference ] [--merge] [--recursive] [--] [...]
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On Fri, Nov 2, 2012 at 3:48 PM, Jeff King 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:j...@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: j...@edge2.net 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On Fri, Nov 2, 2012 at 5:08 PM, Stefano Lattarini 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
On Fri, Nov 2, 2012 at 5:03 PM, Stefano Lattarini
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Lack of netiquette, was Re: [PATCH v4 00/13] New remote-hg helper
On Fri, Nov 2, 2012 at 12:03 PM, Michael J Gruber
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
On 11/02/2012 04:42 PM, Felipe Contreras wrote:
> On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
On Fri, Nov 2, 2012 at 2:58 PM, Stefano Lattarini
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
>> ---
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
On Fri, Nov 2, 2012 at 2:55 PM, Stefano Lattarini
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
On Fri, Nov 2, 2012 at 4:19 PM, Jeff King 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
On Fri, Nov 2, 2012 at 2:12 PM, Jeff King 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] update-index/diff-index: use core.preloadindex to improve performance
On Tue, Oct 30, 2012 at 10:50:42AM +0100, karsten.bl...@dcon.de wrote: > 'update-index --refresh' and 'diff-index' (without --cached) don't honor > the core.preloadindex setting yet. Porcelain commands using these (such as > git [svn] rebase) suffer from this, especially on Windows. > > Use read_cache_preload to improve performance. > > Additionally, in builtin/diff.c, don't preload index status if we don't > access the working copy (--cached). > > Results with msysgit on WebKit repo (2GB in 200k files): > > | update-index | diff-index | rebase > +--++- > msysgit-v1.8.0 | 9.157s |10.536s | 42.791s > + preloadindex | 9.157s |10.536s | 28.725s > + this patch| 2.329s | 2.752s | 15.152s > + fscache [1] | 0.731s | 1.171s | 8.877s Cool numbers. On my quad-core SSD Linux box, I saw a few speedups, too. Here are the numbers for "update-index --refresh" on the WebKit repo (all are wall clock time, best-of-five): | before | after ---++ cold cache | 4.513s | 2.059s warm cache | 0.252s | 0.164s Not as dramatic, but still nice. I wonder how a spinning disk would fare on the cold-cache case, though. I also tried it with all but one CPU disabled, and the warm cache case was a little bit slower. 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. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: avoid full path to store test results
On Fri, Nov 02, 2012 at 04:17:27PM +0100, Felipe Contreras wrote: > > And me, who is trying to figure out what to do with this patch. It is > > presented on its own, outside of a series, with only the description "no > > reason not to do this". > > Yeah, because I think it stands on its own. But I'll include it in the > remote-hg patch series, I already have it queued up. Thanks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
On Fri, Nov 02, 2012 at 04:17:14PM +0100, Johannes Schindelin wrote: > > If so, then this series isn't regressing behavior; the only downside is > > that it's an incomplete fix. In theory this could get in the way of the > > full fix later on, but given the commit messages and the archive of this > > discussion, it would be simple enough to revert it later in favor of a > > more full fix. Is that accurate? > > From my understanding, yes. > > > Sorry if I am belaboring the discussion. I just want to make sure I > > understand the situation before deciding what to do with the topic. It > > sounds like the consensus at this point is "not perfect, but good enough > > to make forward progress". > > I appreciate that stance very much. The patch Sverre and I proposed was > also an incomplete fix (although I suspect it would fix the issue you > pointed out above), so I agree with the "perfect is the enemy of the good" > approach, obviously. Thanks for the response. > 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? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: avoid full path to store test results
On Fri, Nov 2, 2012 at 2:17 PM, Jeff King wrote: > On Wed, Oct 31, 2012 at 07:02:48PM +0100, Johannes Sixt wrote: > >> Am 31.10.2012 03:28, schrieb Felipe Contreras: >> > On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder >> > wrote: >> >> Felipe Contreras wrote: >> >> >> >>> It's all fun and games to write explanations for things, but it's not >> >>> that easy when you want those explanations to be actually true, and >> >>> corrent--you have to spend time to make sure of that. >> >> >> >> That's why it's useful for the patch submitter to write them, asking >> >> for help when necessary. >> >> >> >> As a bonus, it helps reviewers understand the effect of the patch. >> >> Bugs averted! >> > >> > Yeah, that would be nice. Too bad I don't have that information, and >> > have _zero_ motivation to go and get it for you. >> >> Just to clarify: That information is not just for Jonathan, but for >> everyone on this list and those who dig the history a year down the >> road. Contributors who have _zero_ motiviation to find out that >> information are not welcome here because they cause friction and take >> away time from many others for _zero_ gain. > > And me, who is trying to figure out what to do with this patch. It is > presented on its own, outside of a series, with only the description "no > reason not to do this". Yeah, because I think it stands on its own. But I'll include it in the remote-hg patch series, I already have it queued up. > But AFAICT, it is _required_ for the tests in > the remote-hg series to work. Isn't that kind of an important > motivation? It's not _requied_, you will see that error at the end, and the aggregate results would all be 0, but the tests would still work. > Yet it is not in the commit message, nor does the remote-hg series > indicate that it should be built on top. Or am I wrong that the one is > dependent on the other? They are not dependent. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
Hi Peff, On Fri, 2 Nov 2012, Jeff King 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? > > If so, then this series isn't regressing behavior; the only downside is > that it's an incomplete fix. In theory this could get in the way of the > full fix later on, but given the commit messages and the archive of this > discussion, it would be simple enough to revert it later in favor of a > more full fix. Is that accurate? >From my understanding, yes. > Sorry if I am belaboring the discussion. I just want to make sure I > understand the situation before deciding what to do with the topic. It > sounds like the consensus at this point is "not perfect, but good enough > to make forward progress". I appreciate that stance very much. The patch Sverre and I proposed was also an incomplete fix (although I suspect it would fix the issue you pointed out above), so I agree with the "perfect is the enemy of the good" approach, obviously. 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. Thanks, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Set core.ignorecase globally
On Fri, 2 Nov 2012 19:03:37 +0400 Konstantin Khomoutov 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Set core.ignorecase globally
On Fri, 2 Nov 2012 18:39:26 +0400 Kirill Likhodedov 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: C:\tmp>dir /b foo C:\tmp>rename foo Foo C:\tmp>dir /b Foo C:\tmp>del fOO C:\tmp>dir /b C:\tmp>ver Microsoft Windows XP [Версия 5.1.2600] I don't really know what to make out of this, but I'd not change the defaults until the reasons to do this are not really pressing (and they're not for now). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
Jeff King wrote: > If so, then this series isn't regressing behavior; the only downside is > that it's an incomplete fix. In theory this could get in the way of the > full fix later on, but given the commit messages and the archive of this > discussion, it would be simple enough to revert it later in favor of a > more full fix. Is that accurate? > > Sorry if I am belaboring the discussion. I just want to make sure I > understand the situation before deciding what to do with the topic. It > sounds like the consensus at this point is "not perfect, but good enough > to make forward progress". Patch 1, 2, and 4 are good modulo their descriptions. They should work fine without patch 3. Patch 3 is a regression in comprehensibility. I think we can do better. Maybe all it would take is a less confusing description, and tweaks to the code (to loop over revs->cmdline instead of revs->pending) could come on top. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb.perl: fix %highlight_ext
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
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. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
Some additional input: [mjg@localhost git]$ git commit --author='"is this" ' --allow-empty -m test [detached HEAD 0734308] test Author: is thi...@or.not [mjg@localhost git]$ git show commit 0734308b7bf372227bf9f5b9fd6b4b403df33b9e Author: is thi...@or.not Date: Fri Nov 2 15:45:23 2012 +0100 test -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
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. 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.
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.
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.
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.
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.
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.
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.
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).
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.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Set core.ignorecase globally
Hi, 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. Here is the man entry: core.ignorecase If true, this option enables various workarounds to enable git to work better on filesystems that are not case sensitive, like FAT. For example, if a directory listing finds "makefile" when git expects "Makefile", git will assume it is really the same file, and continue to remember it as "Makefile". The default is false, except git-clone(1) or git-init(1) will probe and set core.ignorecase true if appropriate when the repository is created. 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). -- Kirill Likhodedov JetBrains, Inc http://www.jetbrains.com "Develop with pleasure!" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFD: fast-import is picky with author names (and maybe it should - but how much so?)
It seems that our fast-import is super picky with regards to author names. I've encountered author names like Foo Bar Foo Bar if the other system's entry does not parse as a git author name, but fast-import does not accept either of Foo Bar "Foo Bar" because of the way it parses for <>. While the above could be easily turned into Foo Bar it would not be a faithful representation of the original commit in the other dvcs. So the question is: - How should we represent botched author entries faithfully? As a cororollary, fast-import may need to change or not. Michael P.S.: Yes, dvcs=hg, and the "earlier" remote-hg helper chokes on these. garbage in crash out :( -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bisect: "Needed a single revision" message is confusing
Daniel Bonniot venit, vidit, dixit 02.11.2012 14:23: > Hi, > > Suppose I'm doing a git bisect, say: > > $ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b0 > > That works fine. The sha1 could also be a substring, as long as it's > unambiguous, e.g.: > > $ git bisect good 8c7a786b6c > > Now if it's ambiguous, I get an error message: > > $ git bisect good 8 > fatal: Needed a single revision > Bad rev input: 8 > > All fine and good. But what if I somehow input a non-existing sha1 (in > my case see [1]), e,g: > > $ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 > fatal: Needed a single revision > Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 > > I understand that technically both "no revision" and "multiple > revisions" qualify as "not a single revision", but they correspond to > quite different situations. I think it would be more helpful to get > different error messages, something like: > > Bad rev input: 8 refers to multiple revisions > Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 does not refer > to a valid revision > > (and avoid outputing the "fatal: Needed a single revision" message). > > Is this a good idea? Anybody can think of better error messages? > > I'm not familiar with the code base at all, but I could give a try at > implementing it, unless it's trivial enough that someone does it > earlier. After a quick look, it looks like either git-bisect itself or > rev-parse would need to be touched, any pointers and hints welcome. > > Cheers, > > Daniel > > [1] if you want to know, I got a sha1 from one repository and used it > in another, which probably should work, except that when using git-svn > they don't. And the "single revision" error message lead me on a > tangent. > The error comes from rev-parse, which is called by bisect. The problem is that git rev-parse deadbeef git rev-parse --verify deadbeef give two very different error messages (if there's no dead beef there), and that "git ref-parse --verify" gives the same error message for non-existing as for ambiguous revs. There are 3 places in builtins/rev-parse.c which call die_no_single_rev(), and at least some of them should probably choose the error message more carefully. Cheers, Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
On Fri, Nov 02, 2012 at 02:55:41PM +0100, Stefano Lattarini wrote: > > --- /dev/null > > +++ b/git-remote-testgit > > @@ -0,0 +1,62 @@ > > +#!/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. No, we can't assume bash. But this is replacing a script written in python, which we also can't assume. So if it were optional, and skipped gracefully when bash was not available, that would be OK. Of course if it could be done in portable bourne shell, that would be even better (I haven't looked closely, but your comments all seem reasonable). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 09/14] remote-testgit: report success after an import
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
> ---
> 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.
> +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
> ;;
> '')
Regards,
Stefano
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 04/14] Add new simplified git-remote-testgit
On 11/02/2012 03:02 AM, Felipe Contreras wrote:
> It's way simpler. It exerceises the same features of remote helpers.
> It's easy to read and understand. It doesn't depend on python.
>
> It does _not_ exercise the python remote helper framework; there's
> another tool and another test for that.
>
> For now let's just copy the old remote-helpers test script, although
> some of those tests don't make sense for this testgit (they still pass).
>
> In addition, this script would be able to test other features not
> currently being tested.
>
> Signed-off-by: Felipe Contreras
> ---
> Documentation/git-remote-testgit.txt | 2 +-
> git-remote-testgit | 62
> t/t5801-remote-helpers.sh| 134
> +++
> 3 files changed, 197 insertions(+), 1 deletion(-)
> create mode 100755 git-remote-testgit
> create mode 100755 t/t5801-remote-helpers.sh
>
> diff --git a/git-remote-testgit b/git-remote-testgit
> new file mode 100755
> index 000..6650402
> --- /dev/null
> +++ b/git-remote-testgit
> @@ -0,0 +1,62 @@
> +#!/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.
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.
> +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.
> +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
> +while read line; do
> +case "$line" in
>
Useless double quoting (my previous observation about Git coding
guidelines applies here as well, of course).
> +capabilities)
> +echo 'import'
> +echo 'export'
> +echo "refspec $refspec"
> +echo "*import-marks $gitmarks"
> +echo "*export-marks $gitmarks"
> +echo
> +;;
> +list)
> +git for-each-ref --format='? %(refname)' 'refs/heads/'
> +head=$(git symbolic-ref HEAD)
> +echo "@$head HEAD"
> +echo
> +;;
> +import*)
> +# read all import lines
> +while true; do
> +ref="${line#* }"
> +refs="$refs $ref"
> +read line
> +test "${line%% *}" != "import" && break
> +done
> +
> +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 | \
> +sed -e "s#refs/heads/#${prefix}/heads/#g"
> +;;
> +export)
> +git fast-import --{import,export}-marks="$testgitmarks" --quiet
>
Here too.
> +echo
> +;;
> +'')
> +exit
>
I'd put an explicit exit status here, for clarity (this is a matter
of personal preference, so feel free to disregard this nit).
> +;;
> +esac
> +done
Regards,
Stefano
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Overlong lines with git-merge --log
- Original Message - > From: "Tim Janik" > Sent: Friday, November 2, 2012 9:24:29 AM > Subject: Re: Overlong lines with git-merge --log > > On 02.11.2012 11:05, Jeff King wrote: > > > Taking just the first line of those often cuts off the useful part. > > It was deemed less bad to show the whole message as a subject rather > > than cut it off arbitrarily. > > Thanks a lot for the explanation, I'm using git directly here, but the > two cases I had indeed lacked this newline. FWIW we use merge --log quite extensively here at the office, and I've developed a habit to skim the extremely long lines and attempt to cut them intelligently (something I don't trust the computer to do for me). Sometimes that means taking the second or third sentence if it's a better summary, sometimes it's just abbreviating the first. Now that merge automatically spawns an editor, this is quite convenient (though it does take a bit longer). Thanks, Stephen -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Overlong lines with git-merge --log
On 02.11.2012 11:05, Jeff King wrote: > Taking just the first line of those often cuts off the useful part. It > was deemed less bad to show the whole message as a subject rather than > cut it off arbitrarily. Thanks a lot for the explanation, I'm using git directly here, but the two cases I had indeed lacked this newline. -- Yours sincerely, Tim Janik --- http://timj.testbit.eu/ - Free software Author -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: fixup misplacement of --no-post-rewrite description
On Fri, Nov 02, 2012 at 01:26:47PM +0100, Andreas Schwab wrote: > In e858af6 (commit: document a couple of options) the description of the > --no-post-rewrite option was put inside the paragraph for the --amend > option. Move it down after the paragraph. > > Signed-off-by: Andreas Schwab Thanks, the modified output looks much more sane. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: avoid full path to store test results
On Wed, Oct 31, 2012 at 07:02:48PM +0100, Johannes Sixt wrote: > Am 31.10.2012 03:28, schrieb Felipe Contreras: > > On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder wrote: > >> Felipe Contreras wrote: > >> > >>> It's all fun and games to write explanations for things, but it's not > >>> that easy when you want those explanations to be actually true, and > >>> corrent--you have to spend time to make sure of that. > >> > >> That's why it's useful for the patch submitter to write them, asking > >> for help when necessary. > >> > >> As a bonus, it helps reviewers understand the effect of the patch. > >> Bugs averted! > > > > Yeah, that would be nice. Too bad I don't have that information, and > > have _zero_ motivation to go and get it for you. > > Just to clarify: That information is not just for Jonathan, but for > everyone on this list and those who dig the history a year down the > road. Contributors who have _zero_ motiviation to find out that > information are not welcome here because they cause friction and take > away time from many others for _zero_ gain. And me, who is trying to figure out what to do with this patch. It is presented on its own, outside of a series, with only the description "no reason not to do this". But AFAICT, it is _required_ for the tests in the remote-hg series to work. Isn't that kind of an important motivation? Yet it is not in the commit message, nor does the remote-hg series indicate that it should be built on top. Or am I wrong that the one is dependent on the other? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly
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? If so, then this series isn't regressing behavior; the only downside is that it's an incomplete fix. In theory this could get in the way of the full fix later on, but given the commit messages and the archive of this discussion, it would be simple enough to revert it later in favor of a more full fix. Is that accurate? Sorry if I am belaboring the discussion. I just want to make sure I understand the situation before deciding what to do with the topic. It sounds like the consensus at this point is "not perfect, but good enough to make forward progress". -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wishlist: git commit --no-edit
[+cc git@vger; please keep discussion on list] On Fri, Nov 02, 2012 at 10:37:27AM -0200, Thiago Farina wrote: > >> > git commit --amend --no-edit > [...] > > Yup, should be working since 1.7.9. > [...] > I doesn't appear in the help either: > > $ git version > git version 1.8.0.rc2 > > $ git commit -h 2>&1 | grep edit > -c, --reedit-message > reuse and edit message from specified commit > -e, --editforce edit of commit Yeah, parse_options doesn't advertise negative forms of boolean options, as they are implied. I don't think it's a big deal, especially now that it is covered in more detail in the manpage. I think changing it would probably involve adding "--no-edit" as a separate entry in the options struct. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] commit: fixup misplacement of --no-post-rewrite description
In e858af6 (commit: document a couple of options) the description of the --no-post-rewrite option was put inside the paragraph for the --amend option. Move it down after the paragraph. Signed-off-by: Andreas Schwab --- Documentation/git-commit.txt | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 9594ac8..28a5aeb 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -193,10 +193,6 @@ OPTIONS current tip -- if it was a merge, it will have the parents of the current tip as parents -- so the current top commit is discarded. - ---no-post-rewrite:: - Bypass the post-rewrite hook. - + -- It is a rough equivalent for: @@ -213,6 +209,9 @@ You should understand the implications of rewriting history if you amend a commit that has already been published. (See the "RECOVERING FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].) +--no-post-rewrite:: + Bypass the post-rewrite hook. + -i:: --include:: Before making a commit out of staged contents so far, -- 1.8.0 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document 'git commit --no-edit' explicitly
On Fri, Nov 02, 2012 at 11:39:30AM +0100, Matthieu Moy wrote: > I was tempted to merge the paragraph with --edit::, but I thought this > may add confusion. The use-cases for --edit and --no-edit are really > different so I went for a separate paragraph, right below the --edit one. Yeah, usually I think it would be better to keep the positive and negative forms together, but the way you have done it does seem a lot more clear to me. I think it is because the default flips based on other options. So you would be stuck writing something like: -e, --edit:: Edit the commit message using `$EDITOR`. This is the default unless the `-F`, `-m`, or `-C` options are used; in those cases, `-e` or `--edit` can be used to invoke the editor. Likewise, `--no-edit` can be used to suppress the editor when using an existing commit message. For example, `git commit --amend --no-edit` amends a commit without changing its commit message. Your split reads much better, IMHO. Thanks. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document 'git commit --no-edit' explicitly
ni...@lysator.liu.se (Niels Möller) writes: > Matthieu Moy writes: > >> +--no-edit:: >> +Use the selected commit message without launching an editor. >> +For example, `git commit --amend --no-edit` amends a commit >> +without changing its commit message. > > Looks clear enough to me. Thanks for fixing. > > Are -c --no-edit and -C really equivalent in all cases? I guess so, but I didn't check. > If so, maybe you want to say that in the documentation for -C. I don't think we should target exhaustivity in the documentation, so I'm not in favor of *adding* it. But maybe we should replace the doc with stg like: -c :: --reedit-message=:: Like '-C --edit'. (it could even make sense to deprecate one of -C/-c) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Document 'git commit --no-edit' explicitly
Matthieu Moy writes: > +--no-edit:: > + Use the selected commit message without launching an editor. > + For example, `git commit --amend --no-edit` amends a commit > + without changing its commit message. Looks clear enough to me. Thanks for fixing. Are -c --no-edit and -C really equivalent in all cases? If so, maybe you want to say that in the documentation for -C. (And if not, maybe they *ought* to be equivalent). Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Lack of netiquette, was Re: [PATCH v4 00/13] New remote-hg helper
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. But even reading pages like these
requires a common sense (of the many undefined terms therein), a sense
which is usually present here on the list, and thus renders a page like
these unnecessary for us. Once there is a lack of commonality, there is
a lack of agreement about those undefined terms (what constitutes a
personal attack etc.).
Consequently, the only practical test for commonality and community
acceptance appears to be just that: commonality and community
acceptance. If many people in a community consider a tone or formulation
offensive, then it is offensive by the very definition of common sense
(common to that community), and there's no point at all in arguing about
it. If I don't like a community's sense I either deal with it or leave it.
It's really not that different from coding style. If we prefer
if (cond) {
over
if (cond)
{
then you either do it that way or your code gets rejected. The
difference is that coding style is easier to define, of course. The
common thing is that there's no point in arguing about it.
Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Document 'git commit --no-edit' explicitly
Signed-off-by: Matthieu Moy --- I was tempted to merge the paragraph with --edit::, but I thought this may add confusion. The use-cases for --edit and --no-edit are really different so I went for a separate paragraph, right below the --edit one. Documentation/git-commit.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 3acf2e7..44b4347 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -188,6 +188,11 @@ OPTIONS commit log message unmodified. This option lets you further edit the message taken from these sources. +--no-edit:: + Use the selected commit message without launching an editor. + For example, `git commit --amend --no-edit` amends a commit + without changing its commit message. + --amend:: Used to amend the tip of the current branch. Prepare the tree object you would want to replace the latest commit as usual -- 1.8.0.319.g8abfee4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wishlist: git commit --no-edit
Matthieu Moy writes: > Err, isn't this already working? (maybe your version of Git is too old, > but my 1.7.9.5 does this at least) It may be that I'm confused. It doesn't work in the version I have (1.7.2.5, debian packaged). And then I also had a quick look in the source code (of a fresh pull from https://github.com/git/git.git). In builtin/commit.c, I didn't see any no-edit flag (but I maybe it's in the OPT_BOOL magic (which is different from OPT_BOOLEAN?)). And it's also not mentioned in Documentation/git-commit.txt or on http://kernel.org/pub/software/scm/git/docs/git-commit.html. If commit --no-edit does work in recent versions, then all is fine, except possibly the documentation of it. Thanks. Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Overlong lines with git-merge --log
On Fri, Nov 02, 2012 at 02:30:15AM +0100, Tim Janik wrote: > Using git-merge --log to merge commit messages that spawn multiple lines > will produce overlong summary lines. > That's because each summary line for a commit includes the entire commit > message (all lines concatenated). > > According to git-fmt-merge-msg(1), --log should 'populate the log > message with one-line descriptions' instead of using the entire commit > messages. > Which would make a lot of sense, it's just not working as advertised. The "subject" or "first line" is not actually the first line; these days it is typically the "first paragraph". The reason is that git always expected commit messages to look like: one line describing the change more detailed explanation that might go on for several lines or even several paragraphs However, as people imported commits from previous systems, they ended up with a lot of commit messages where the closest thing to a subject was split across several lines like: here's a description of the commit that is probably overly long, but that's how we did it back in CVS days Taking just the first line of those often cuts off the useful part. It was deemed less bad to show the whole message as a subject rather than cut it off arbitrarily. If you are developing with git and not splitting the subject out with a blank line, there are a lot of tools that are going to look ugly (e.g., format-patch will generate overly long subjects, "log --oneline" will be ugly, and browsers like "tig" and "gitk" may be overwhelmed). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wishlist: git commit --no-edit
On Fri, Nov 02, 2012 at 10:42:24AM +0100, Matthieu Moy wrote: > ni...@lysator.liu.se (Niels Möller) writes: > > > I'd like to have a git commit option which is the opposite of --edit, to > > use the selected commit message as is, without invoking any editor. > > > > Main use case I see would be > > > > git commit --amend --no-edit > > Err, isn't this already working? (maybe your version of Git is too old, > but my 1.7.9.5 does this at least) Yup, should be working since 1.7.9. $ git tag --contains ':/commit: honour --no-edit' | sort -V | head -1 v1.7.9 -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2012, #09; Mon, 29)
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.
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?
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;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2012, #09; Mon, 29)
On Thu, Nov 01, 2012 at 08:12:20PM -0500, Chris Rorvick wrote:
> > Just FYI, t9604-cvsimport-timestamps.sh is still failing for me.
> >
> > I haven't spent too long on this yet, but I had hoped that setting
> > TZ would sidestep any DST issues. (I have downloaded new tzdata, but
> > have yet to install - actually I don't really want to!). It is not
> > clear from the tzset manpage what happens if you use the DST format
> > for TZ, but you don't provide the start/end date for DST, which is
> > what this test is doing.
> >
> > Perhaps the test should use the non-DST format? e.g. "TZ=CST6 git ..."
> > Does the test really care about DST? (*if* that is indeed the problem).
>
> It actually looks like your TZ database is fine and the problem is
> with the conversion to a struct tm. In each case, the time is
> localized to the previous TZ value while the offset for the current TZ
> value. For example, look at the first commit in the first test. It
> converted the timestamp to 18:00 (CST6) while all the rest came
> through as expected.I suspect the previous version of cvsimport
> would exhibit similar behavior with the first imported commit. What
> is your platform?
Yeah, I think that is it. IIRC, Ramsay is on cygwin, and I noticed this
in perl 5.16's POSIX.xs:
#ifdef WIN32
/*
* (1) The CRT maintains its own copy of the environment, separate from
* the Win32API copy.
*
* (2) CRT getenv() retrieves from this copy. CRT putenv() updates this
* copy, and then calls SetEnvironmentVariableA() to update the Win32API
* copy.
*
* (3) win32_getenv() and win32_putenv() call GetEnvironmentVariableA() and
* SetEnvironmentVariableA() directly, bypassing the CRT copy of the
* environment.
*
* (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}.
*
* Therefore, we need to retrieve the value of $ENV{TZ} and call CRT
* putenv() to update the CRT copy of the environment (if it is different)
* whenever we're about to call tzset().
[...]
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.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wishlist: git commit --no-edit
ni...@lysator.liu.se (Niels Möller) writes: > I'd like to have a git commit option which is the opposite of --edit, to > use the selected commit message as is, without invoking any editor. > > Main use case I see would be > > git commit --amend --no-edit Err, isn't this already working? (maybe your version of Git is too old, but my 1.7.9.5 does this at least) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Lack of netiquette, was Re: [PATCH v4 00/13] New remote-hg helper
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.
--
Andreas Ericsson andreas.erics...@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Wishlist: git commit --no-edit
I'd like to have a git commit option which is the opposite of --edit, to use the selected commit message as is, without invoking any editor. Main use case I see would be git commit --amend --no-edit (say you fix a typo in the previous commit which doesn't affect the commit message). Today, one can get the same effect more clumsily as EDITOR=touch git commit --amend Or maybe as git commit --amend -C HEAD But I'd really prefer a --no-edit option over those alternatives. And it might be useful also in combination with other options, e.g., it would make the -C option equivalent to -c --no-edit, if I understand it correctly. Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
