* Re: Candidates for sequencer?
From: Jeff King @ 2012-11-12 22:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0n0tTZ3EgzeesOr2B1LjeEUjTnWQh5dDfW28jA1ia0-gA@mail.gmail.com>
On Tue, Nov 13, 2012 at 03:16:51AM +0530, Ramkumar Ramachandra wrote:
> I'd like to get more commands to call into sequencer.c, so that we can
> (finally) implement a sensible `git continue` and `git abort`.
> Currently, am and rebase come to mind, but they are shell scripts and
> I don't see how we can make them call into sequencer.c. Is there any
> way to make sequencer.c more useful? Should we implement a `git
> continue` and `git reset` anyway, lifting code from 83c750ac to check
> the tree state?
I'd love it if ther was a mechanism for interruptible commands to notify
the rest of git that they are in progress. This would not only make
cross-language commands like "rebase" and "am" work easily, but it would
mean that we can automatically support third-party commands that we
don't even know about.
I am already doing something like this for some personal scripts which
do sequences of merges and rebases, and my "git continue" looks like:
if test -f "$GIT_DIR/continue"; then
eval "$(cat "$GIT_DIR/continue")"
elif test -d "$GIT_DIR/rebase-merge"; then
git rebase --continue
elif test -d "$GIT_DIR/rebase-apply"; then
if test -f "$GIT_DIR/rebase-apply/applying"; then
git am --continue
else
git rebase --continue
fi
fi
[... etc ...]
It works pretty well in practice, but I'd have a few suggestions on top:
1. Even if "am" and "rebase" learn to use $GIT_DIR/continue, we should
probably continue to support the fallback state-examination in case
people use mixed versions of git. And clearly the code should be
shared with what's in wt-status.c.
2. The "continue" file should probably be called "in-progress", and
should have more structured data, like the name of the operation.
3. The list of operations in progress needs to be a stack. If I am
rebasing and start a cherry pick, what would "git continue" do? It
probably makes sense to continue the most recently started
operation first.
4. Rebase already detects an in-progress rebase. It should probably
also detect an "am" or "cherry-pick" in progress. But it's not
commutative; you should be able to start a "cherry-pick" in the
middle of a "rebase". This is really independent of the storage
format (we could do it right now if the state logic from wt-status
is made available in a machine-readable format).
We _might_ want to put information in the in-progress file that
helps programs reach that decision. I'm not sure of a sane way of
doing it, though. You could say "don't rebase while I'm in
progress", but that depends on the writer knowing the universe of
all programs that might be run. It might be possible to come up
with some taxonomy of operations like "a history rewriting
operation is in progress" which would cause other history rewriting
operations to choose not to start. But coming up with the right set
of attributes might be impossible.
-Peff
^ permalink raw reply
* Re: [PATCH] custom log formats for "diff --submodule=log"
From: Jeff King @ 2012-11-12 22:19 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Jeffrey S. Haemer
In-Reply-To: <CALkWK0nT8RXXUUkLuS8zWVMG=wsm0b740q-qYb3iN3cO7Tb=0A@mail.gmail.com>
On Sun, Nov 11, 2012 at 06:35:16PM +0530, Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra wrote:
> > Don't you mean `git diff` in place of `git log -p`
> > though? I don't think `git log --submodule` does anything differently
> > from `git log`.
>
> Sorry for the nonsense. I just realized that it affects the diffs
> shown by `git log -p`.
Right. I don't know if that is a common workflow (I do not really use
submodules), but it was the first thing I tried when I heard about
--submodule (and it does make sense to me that it would work).
But the logic could apply equally well to:
git diff --submodule=log:%an
-Peff
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jean-Jacques Lafay @ 2012-11-12 22:27 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, msysgit, Git List, Philip Oakley
In-Reply-To: <20121111165431.GA25884@sigill.intra.peff.net>
2012/11/11 Jeff King <peff@peff.net>:
> On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
>
> Ultimately, I have some ideas for doing this in a breadth-first way,
> which would make it more naturally iterative. It would involve having N
> bits of storage per commit to check N tags, but it would mean that we
> could get accurate answers in the face of clock skew (like the
> merge-base calculation, it would merely get slower in the face of skew).
I guess the optimal algorithm may also depend on the commit graph
general shape, but intuitively, I'd say that the critical factor is
the number and distribution of tags. As soon as you have a significant
number of tags (let's say 1% of the commits are tagged, evenly
distributed), you'll quickly end up with every commit marked as
containing or not the target commit, so that each additional tag check
is cheap.
This suggests a complexity of O(number of commits) more often then
not, however you choose to traverse the graph.
At least on my almost-real-life repo*, with ~140,000 commits and
~2,000 tags, tag --contains takes a few seconds, of course more than
the worst-case test on a 40,000 commit / 1 tag repo, but still in the
same order of magnitude.
* : meaning we're in the process of migrating from clearcase (deep
sighs allowed !), and for now I kept almost everything in a single git
repo, which may not be the eventual choice
Jean-Jacques.
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-12 22:47 UTC (permalink / raw)
To: Jeff King; +Cc: A Large Angry SCM, Michael J Gruber, Git Mailing List
In-Reply-To: <20121112214127.GA10531@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 10:41 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 07:48:14PM +0100, Felipe Contreras wrote:
>
>> > 3. Exporters should not use it if they have any broken-down
>> > representation at all. Even knowing that the first half is a human
>> > name and the second half is something else would give it a better
>> > shot at cleaning than fast-import would get.
>>
>> I'm not sure what you mean by this. If they have name and email, then
>> sure, it's easy.
>
> But not as easy as just printing it. What if you have this:
>
> name="Peff <angle brackets> King"
> email="<peff@peff.net>"
>
> Concatenating them does not produce a valid git author name. Sending the
> concatenation through fast-import's cleanup function would lose
> information (namely, the location of the boundary between name and
> email).
Right. Unfortunately I'm not aware of any DSCM that does that.
> Similarly, one might have other structured data (e.g., CVS username)
> where the structure is a useful hint, but some conversion to name+email
> is still necessary.
CVS might be the only one that has such structured data. I think in
subversion the username has no meaning. A 'felipec' subversion
username is as bad as a mercurial 'felipec' username.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 22:52 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Felipe Contreras, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CALkWK0mRKznNN7750h=k6aE3OJ7hBLVC-G51gEYi2+NuYjPWrQ@mail.gmail.com>
On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > cc-cmd is only per-file, and many times receipients get lost without
> > seing the full patch series.
>
> s/seing/seeing
>
> > [...]
>
> Looks good otherwise.
s/receipients/recipients/ too
Practically this is ok but I think it's unnecessary.
Output from git format-patch is always in a single
directory.
My work flow is to use a script for --to and --cc
lines that can be set to emit the same addresses for
all files in a patch series or generate different
addresses per patch file.
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-12 23:03 UTC (permalink / raw)
To: Joe Perches
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <1352760759.18715.7.camel@joe-AO722>
On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>>
>> s/seing/seeing
>>
>> > [...]
>>
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.
A temporary directory.
> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.
For --to-cmd and --cc-cmd? So basically you check the dirname of the
argument passed?
While that works, it means you have to run the same command multiple
times, one for each mail.
If the command is using something expensive such as 'git blame' and
you have many patches, this is particularly bad. Also, it's not
elegant :)
--
Felipe Contreras
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Andreas Schwab @ 2012-11-12 23:09 UTC (permalink / raw)
To: Drew Northup; +Cc: glpk xypron, git, jnareb, Junio C Hamano
In-Reply-To: <CAM9Z-n=6xsC7yiKJ+NU-CxNPxEXWmJzvXLUocgZgWPQnuK6G4Q@mail.gmail.com>
Drew Northup <n1xim.email@gmail.com> writes:
> Something like this may be useful to defuse the "file" parameter, but
> I presume a more definitive fix is in order...
A proper fix will have to add esc_html to the feed generation, something
like this (untested):
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10ed9e5..a51a8ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -8055,6 +8055,7 @@ sub git_feed {
$feed_type = 'history';
}
$title .= " $feed_type";
+ $title = esc_html($title);
my $descr = git_get_project_description($project);
if (defined $descr) {
$descr = esc_html($descr);
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: splitting off shell test framework
From: Adam Spiers @ 2012-11-12 23:09 UTC (permalink / raw)
To: Drew Northup; +Cc: Git Mailing List, felipe.contreras, Jason J Pyeron CTR (US)
In-Reply-To: <CAM9Z-n=ypt_fyFcPjYD28NNoxnJ2ZSovd1TbERdA8RJY1Va=kA@mail.gmail.com>
On Mon, Nov 12, 2012 at 6:18 PM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Mon, Nov 12, 2012 at 11:37 AM, Adam Spiers <git@adamspiers.org> wrote:
>> As it turned out to be fairly easy, I was wondering if there would be
>> any interest in doing this more formally, i.e. splitting off the
>> framework so that it could be used and improved outside the scope of
>> git development? Of course this would pose the question how git would
>> consume this new project without any risk of destabilisation. I'm
>> guessing that simply using a git submodule would solve the problem,
>> but ICBW ...
>>
>> Just an idea. Interesting, or terrible? :)
>
> Done at least once already:
>
> http://comments.gmane.org/gmane.comp.version-control.git/201591
Nice! So hopefully someone will submit patches to build a two-way bridge
via git subtree. Having them diverge would be sad.
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 23:13 UTC (permalink / raw)
To: Felipe Contreras
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CAMP44s0B42yt8eugYSjYweoX8WOXXF1f-0N-5FuUyrY5rgCiow@mail.gmail.com>
On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
> >> Felipe Contreras wrote:
> >> > cc-cmd is only per-file, and many times receipients get lost without
> >> > seing the full patch series.
> >>
> >> s/seing/seeing
> >>
> >> > [...]
> >>
> >> Looks good otherwise.
> >
> > s/receipients/recipients/ too
> >
> > Practically this is ok but I think it's unnecessary.
> >
> > Output from git format-patch is always in a single
> > directory.
>
> A temporary directory.
>
> > My work flow is to use a script for --to and --cc
> > lines that can be set to emit the same addresses for
> > all files in a patch series or generate different
> > addresses per patch file.
>
> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> argument passed?
yes. basename and dirname
> While that works, it means you have to run the same command multiple
> times, one for each mail.
Shrug. it's not a generally significant cost.
The script could also output the addresses to yet another file.
> If the command is using something expensive such as 'git blame' and
> you have many patches, this is particularly bad. Also, it's not
> elegant :)
Elegant is a beholder viewpoint.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] git tag --contains : avoid stack overflow
From: Jeff King @ 2012-11-12 23:14 UTC (permalink / raw)
To: Jean-Jacques Lafay; +Cc: René Scharfe, msysgit, Git List, Philip Oakley
In-Reply-To: <CA+TMuX2p4ck0qXijH+OOcBoveBC42U8PqnXdisau57RXwt1isg@mail.gmail.com>
On Mon, Nov 12, 2012 at 11:27:14PM +0100, Jean-Jacques Lafay wrote:
> 2012/11/11 Jeff King <peff@peff.net>:
> > On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
> >
> > Ultimately, I have some ideas for doing this in a breadth-first way,
> > which would make it more naturally iterative. It would involve having N
> > bits of storage per commit to check N tags, but it would mean that we
> > could get accurate answers in the face of clock skew (like the
> > merge-base calculation, it would merely get slower in the face of skew).
>
> I guess the optimal algorithm may also depend on the commit graph
> general shape, but intuitively, I'd say that the critical factor is
> the number and distribution of tags. As soon as you have a significant
> number of tags (let's say 1% of the commits are tagged, evenly
> distributed), you'll quickly end up with every commit marked as
> containing or not the target commit, so that each additional tag check
> is cheap.
>
> This suggests a complexity of O(number of commits) more often then
> not, however you choose to traverse the graph.
We can do much better than O(number of commits), though, if we stop
traversing down a path when its timestamp shows that it is too old to
contain the commits we are searching for. The problem is that the
timestamps cannot always be trusted, because they are generated on
machines with wrong clocks, or by buggy software. This could be solved
by calculating and caching a "generation" number, but last time it was
discussed there was a lot of arguing and nothing got done.
Another approach, used by the merge-base calculation, is to treat
parents in a breadth-first way, but sort them by timestamp, and walk
backwards to find the merge-base (and if you get to a merge-base, you
can stop). In that case, bad timestamps may cause you to look at extra
commits (because you process a commit prematurely and end up going
deeper than the merge base), but it can never give you a wrong answer.
Thinking on it more, though, the merge-base style of computation would
mean you always have to walk backwards to the oldest tag. Which is in
the same order of magnitude as the number of commits, assuming you have
tags near the start of history. So I think we will always want to keep a
cutoff, anyway, and there is not much point in switching off of a
depth-first approach (but of course it does make sense to use iteration
instead of recursion to do so).
> At least on my almost-real-life repo*, with ~140,000 commits and
> ~2,000 tags, tag --contains takes a few seconds, of course more than
> the worst-case test on a 40,000 commit / 1 tag repo, but still in the
> same order of magnitude.
Try "git rev-list --count --all" to get a sense of how long O(# of
commits) takes. Before the depth-first implementation, "tag --contains"
was O(commits * tags) in the worst case. With depth first, it's
O(commits), and then with the timestamp cutoff, it's really O(commits
since needle), where "needle" is the oldest thing you're looking for.
Here are those numbers on linux-2.6 (with linux-stable tags):
[to get a sense of the repo size]
$ git for-each-ref refs/tags | wc -l
909
$ git rev-list --count --all
363413
[this is O(commits)]
$ time git rev-list --count --all
real 0m4.034s
user 0m3.960s
sys 0m0.056s
[before depth-first, ffc4b80^; you can see that it is much worse than
O(commits), though not as bad as the worst case (because finding the
merge bases for recent tags is not O(commits)]
$ time git tag --contains HEAD~200
real 0m42.838s
user 0m42.527s
sys 0m0.156s
[after depth-first, ffc4b80; you can see that this is O(commits),
because we will go depth-first down to the roots, but do only a
single traversal]
$ time git tag --contains HEAD~200
real 0m3.939s
user 0m3.784s
sys 0m0.140s
[with my generation patches; much faster]
$ time git tag --contains HEAD~200
real 0m0.037s
user 0m0.020s
sys 0m0.012s
I was thinking we had merged the timestamp cutoff (which has the same
performance characteristics as generations, just with the skew issue) to
master, but it looks like we didn't.
-Peff
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: git log to use .mailmap by default?
From: Jeff King @ 2012-11-12 23:18 UTC (permalink / raw)
To: Jason Timrod; +Cc: git@vger.kernel.org
In-Reply-To: <1352635999.79618.YahooMailNeo@web160306.mail.bf1.yahoo.com>
On Sun, Nov 11, 2012 at 04:13:19AM -0800, Jason Timrod wrote:
> I note that when defining a .mailmap file, that it's honoured by
> git-shotlog(1) by default, but for git-log(1) I have to define an
> entirely new --pretty= formatting option to use it.
>
> Why is this? Why doesn't git-log honour this by default like
> git-shortlog does?
>
> Would there be a way of making .mailmap be the default without needing
> a pretty option each time to redefine the default?
I think it's mostly hysterical raisins. Mailmap grew as a shortlog
feature, and then only later became available via custom format
specifiers like "%aN", in git-blame, etc. And nobody ever updated the
stock formats.
It would make sense to me to add an option to do so, and probably to
turn it on by default for normal human-consumed formats, but probably
not for "--pretty=raw".
-Peff
^ permalink raw reply
* Re: [PATCH] gitweb: git_summary - show $project in title
From: Jeff King @ 2012-11-12 23:25 UTC (permalink / raw)
To: Henrich Schuchardt; +Cc: git
In-Reply-To: <1352611258-11450-1-git-send-email-xypron.glpk@gmx.de>
On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
> Gitweb pages are structured by divs of class title with grey background.
> The shortlog, and the log page show the project name as the first title.
> Page summary only shows an empty grey box above the project details.
> This provides an inconstent user experience.
>
> This patch adds the missing project title.
>
> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
> ---
> gitweb/gitweb.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..3e1c452 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6451,7 +6451,7 @@ sub git_summary {
> git_header_html();
> git_print_page_nav('summary','', $head);
>
> - print "<div class=\"title\"> </div>\n";
> + print "<div class=\"title\">$project</div>\n";
I do not have any opinion on whether the intent of the change is good or
not, but shouldn't $project be run through esc_html() here?
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-12 23:35 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <1352653610-2090-1-git-send-email-felipe.contreras@gmail.com>
On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:
> There's no point in asking this over and over if the user already
> properly configured his/her name and email.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>
> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
It should be defaulting to your regular git ident, and you just have to
hit enter, right?
I think it's probably reasonable to skip that "enter" in most cases. But
I'm not sure why we ever asked in the first place. What do people input
there if they are not taking the default?
> diff --git a/git-send-email.perl b/git-send-email.perl
> index aea66a0..65b8328 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -748,6 +748,17 @@ if (!$force) {
> }
> }
>
> +my $name = Git::config('user.name');
> +my $email = Git::config('user.email');
> +
> +if (defined $email) {
> + if (defined $name) {
> + $sender = sprintf("%s <%s>", $name, $email);
> + } else {
> + $sender = $email;
> + }
> +}
Why not use Git::ident_person() here? It saves some code, and would also
respect environment variables. Or better yet...
> my $prompting = 0;
> if (!defined $sender) {
> $sender = $repoauthor || $repocommitter || '';
Why not just use $repoauthor or $repocommitter, as the prompt default
already does?
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-12 23:37 UTC (permalink / raw)
To: Joe Perches
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <1352761982.18715.20.camel@joe-AO722>
On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
>> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> >> Felipe Contreras wrote:
>> >> > cc-cmd is only per-file, and many times receipients get lost without
>> >> > seing the full patch series.
>> >>
>> >> s/seing/seeing
>> >>
>> >> > [...]
>> >>
>> >> Looks good otherwise.
>> >
>> > s/receipients/recipients/ too
>> >
>> > Practically this is ok but I think it's unnecessary.
>> >
>> > Output from git format-patch is always in a single
>> > directory.
>>
>> A temporary directory.
>>
>> > My work flow is to use a script for --to and --cc
>> > lines that can be set to emit the same addresses for
>> > all files in a patch series or generate different
>> > addresses per patch file.
>>
>> For --to-cmd and --cc-cmd? So basically you check the dirname of the
>> argument passed?
>
> yes. basename and dirname
Well, the basename is irrelevant, because you don't care witch
particular patch is being sent, you are going to process all of them
every time.
>> While that works, it means you have to run the same command multiple
>> times, one for each mail.
>
> Shrug. it's not a generally significant cost.
It is when you use 'git blame' and there's a lot of history:
% time git cc-cmd -1 1266686
Peter Krefting <peter@softwolves.pp.se>
junio
Tran Ngoc Quan <vnwildman@gmail.com>
Jiang Xin <worldhello.net@gmail.com>
git cc-cmd -1 1266686 0.23s user 0.16s system 1% cpu 19.991 total
> The script could also output the addresses to yet another file.
Which would be even more hacky.
>> If the command is using something expensive such as 'git blame' and
>> you have many patches, this is particularly bad. Also, it's not
>> elegant :)
>
> Elegant is a beholder viewpoint.
You think a solution that runs the same instructions multiple times
unnecessarily is elegant? All right.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Joe Perches @ 2012-11-12 23:40 UTC (permalink / raw)
To: Felipe Contreras
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Jonathan Nieder,
Pascal Obry
In-Reply-To: <CAMP44s2EyMcYHYGvxZnj90qD5ms2YZ5O6gXv_TTES0H72vUm0g@mail.gmail.com>
On Tue, 2012-11-13 at 00:37 +0100, Felipe Contreras wrote:
> On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
[]
> >> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> >> argument passed?
> >
> > yes. basename and dirname
>
> Well, the basename is irrelevant, because you don't care witch
> particular patch is being sent, you are going to process all of them
> every time.
Well, I do different actions on cover letter patches
than other patches because for patch sets that touch
lots of files, the cc list can be _very_ long and
that can run afoul of other issues like maximum
recipient counts for various mailing lists.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-12 23:42 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121112233546.GG10531@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 12:35 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:
>
>> There's no point in asking this over and over if the user already
>> properly configured his/her name and email.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
>
> It should be defaulting to your regular git ident, and you just have to
> hit enter, right?
Yes.
> I think it's probably reasonable to skip that "enter" in most cases. But
> I'm not sure why we ever asked in the first place. What do people input
> there if they are not taking the default?
Beats me.
> Why not use Git::ident_person() here? It saves some code, and would also
> respect environment variables. Or better yet...
I assume there was a reason why that code was asking for input;
precisely because it would use the environment variables. For some
reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
is not right, or the full name config.
OTOH user.name/.email configurations come clearly from the user.
>> my $prompting = 0;
>> if (!defined $sender) {
>> $sender = $repoauthor || $repocommitter || '';
>
> Why not just use $repoauthor or $repocommitter, as the prompt default
> already does?
See above.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 0:02 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <CAMP44s0emar-C27SX-FDsUVB6Sevuy4fBFHuO2OD6xELCEjmGg@mail.gmail.com>
On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:
> > Why not use Git::ident_person() here? It saves some code, and would also
> > respect environment variables. Or better yet...
>
> I assume there was a reason why that code was asking for input;
> precisely because it would use the environment variables. For some
> reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
> is not right, or the full name config.
>
> OTOH user.name/.email configurations come clearly from the user.
But we use the environment to default the field, so the distinction
doesn't make much sense to me. Plus, it has always been the case that
you can use git without setting user.*, but instead only using the
environment. I don't see any reason not to follow that principle here,
too.
The one distinction that would make sense to me is pausing to ask when
we use "implicit" methods to look up the ident, like concatenating the
username with the hostname to get the email.
Git::ident uses "git var" to do its lookup, which will use IDENT_STRICT;
that stops most junk like empty names and bogus domains. But I think we
would want to go one step further and actually check
user_ident_sufficiently_given. Unfortunately, that is not currently
available outside of C. You'd probably want something like:
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..eaf324e 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -26,6 +26,12 @@ static const char *pager(int flag)
return pgm;
}
+static const char *explicit_ident(int flag)
+{
+ git_committer_info(flag);
+ return user_ident_sufficiently_given() ? "1" : "0";
+}
+
struct git_var {
const char *name;
const char *(*read)(int);
@@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
{ "GIT_AUTHOR_IDENT", git_author_info },
{ "GIT_EDITOR", editor },
{ "GIT_PAGER", pager },
+ { "GIT_EXPLICIT_IDENT", explicit_ident },
{ "", NULL },
};
-Peff
^ permalink raw reply related
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 0:06 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113000217.GH10531@sigill.intra.peff.net>
On Mon, Nov 12, 2012 at 07:02:17PM -0500, Jeff King wrote:
> The one distinction that would make sense to me is pausing to ask when
> we use "implicit" methods to look up the ident, like concatenating the
> username with the hostname to get the email.
By the way, I suspect this is the answer to "what do people type for
this prompt". It is probably more about a safety on bad ident than it is
about people routinely updating the information. I actually think it
would make more sense to drop the prompt entirely and just die when the
user has not given us a usable ident. But maybe people who do one-off
format-patches would rather type their name in a prompt than set an
environment variable and re-run the program.
-Peff
^ permalink raw reply
* Re: Test failures in contrib/remote-helpers
From: Felipe Contreras @ 2012-11-13 0:35 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0k9trxx8NC1GWw-yYzBKhFchrvg2JLeBtyoAkokmv9A0w@mail.gmail.com>
On Sun, Nov 11, 2012 at 1:48 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Felipe Contreras wrote:
>> On Sun, Nov 11, 2012 at 11:32 AM, Ramkumar Ramachandra
>> <artagnon@gmail.com> wrote:
>>> I'm experiencing test failures in contrib/remote-helpers.
>>
>> Which are your versions of hg, and bzr?
>
> Mercurial Distributed SCM (version 1.9.1)
> Bazaar (bzr) 2.4.1
And here's the one for bzr:
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -646,12 +646,12 @@ def get_repo(url, alias):
global dirname, peer
clone_path = os.path.join(dirname, 'clone')
- origin = bzrlib.controldir.ControlDir.open(url)
+ origin = bzrlib.bzrdir.BzrDir.open(url)
remote_branch = origin.open_branch()
if os.path.exists(clone_path):
# pull
- d = bzrlib.controldir.ControlDir.open(clone_path)
+ d = bzrlib.bzrdir.BzrDir.open(clone_path)
branch = d.open_branch()
result = branch.pull(remote_branch, [], None, False)
else:
I have other fixes to run up to 2.0.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Junio C Hamano @ 2012-11-13 0:37 UTC (permalink / raw)
To: Joe Perches
Cc: Ramkumar Ramachandra, Felipe Contreras, git, Jonathan Nieder,
Pascal Obry
In-Reply-To: <1352760759.18715.7.camel@joe-AO722>
Joe Perches <joe@perches.com> writes:
> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>>
>> s/seing/seeing
>>
>> > [...]
>>
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.
Sorry, but I do not see how the usefulness (or necessity) of this
new option is connected to the fact that you can tell the command to
write the patches into a single (possibly new) directory. Care to
explain?
> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.
^ permalink raw reply
* Re: [PATCH] gitweb: git_summary - show $project in title
From: Junio C Hamano @ 2012-11-13 0:46 UTC (permalink / raw)
To: Jeff King; +Cc: Henrich Schuchardt, git
In-Reply-To: <20121112232513.GF10531@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Nov 11, 2012 at 06:20:58AM +0100, Henrich Schuchardt wrote:
>
>> Gitweb pages are structured by divs of class title with grey background.
>> The shortlog, and the log page show the project name as the first title.
>> Page summary only shows an empty grey box above the project details.
>> This provides an inconstent user experience.
>>
>> This patch adds the missing project title.
>>
>> Signed-off-by: Henrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> gitweb/gitweb.perl | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 10ed9e5..3e1c452 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -6451,7 +6451,7 @@ sub git_summary {
>> git_header_html();
>> git_print_page_nav('summary','', $head);
>>
>> - print "<div class=\"title\"> </div>\n";
>> + print "<div class=\"title\">$project</div>\n";
>
> I do not have any opinion on whether the intent of the change is good or
> not, but shouldn't $project be run through esc_html() here?
I think the answer is yes. And if $project needs to be escaped, the
git_feed function you fixed today has another codepath that needs to
be fixed. When git_get_project_description($project) returns undef,
the description is taken from $project without any escaping.
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Felipe Contreras @ 2012-11-13 0:54 UTC (permalink / raw)
To: Jeff King; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113000217.GH10531@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 1:02 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:
>
>> > Why not use Git::ident_person() here? It saves some code, and would also
>> > respect environment variables. Or better yet...
>>
>> I assume there was a reason why that code was asking for input;
>> precisely because it would use the environment variables. For some
>> reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
>> is not right, or the full name config.
>>
>> OTOH user.name/.email configurations come clearly from the user.
>
> But we use the environment to default the field, so the distinction
> doesn't make much sense to me. Plus, it has always been the case that
> you can use git without setting user.*, but instead only using the
> environment. I don't see any reason not to follow that principle here,
> too.
And that's why a lot of commits end up like michael
<michael@michael-laptop.(none)>.
> The one distinction that would make sense to me is pausing to ask when
> we use "implicit" methods to look up the ident, like concatenating the
> username with the hostname to get the email.
>
> Git::ident uses "git var" to do its lookup, which will use IDENT_STRICT;
> that stops most junk like empty names and bogus domains. But I think we
> would want to go one step further and actually check
> user_ident_sufficiently_given. Unfortunately, that is not currently
> available outside of C. You'd probably want something like:
>
> diff --git a/builtin/var.c b/builtin/var.c
> index aedbb53..eaf324e 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -26,6 +26,12 @@ static const char *pager(int flag)
> return pgm;
> }
>
> +static const char *explicit_ident(int flag)
> +{
> + git_committer_info(flag);
> + return user_ident_sufficiently_given() ? "1" : "0";
> +}
> +
> struct git_var {
> const char *name;
> const char *(*read)(int);
> @@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
> { "GIT_AUTHOR_IDENT", git_author_info },
> { "GIT_EDITOR", editor },
> { "GIT_PAGER", pager },
> + { "GIT_EXPLICIT_IDENT", explicit_ident },
> { "", NULL },
> };
Probably. But what I really want is to stop 'git send-email' from
asking. I think the one next step further can be done later.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Junio C Hamano @ 2012-11-13 0:55 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <20121113000637.GI10531@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 12, 2012 at 07:02:17PM -0500, Jeff King wrote:
>
>> The one distinction that would make sense to me is pausing to ask when
>> we use "implicit" methods to look up the ident, like concatenating the
>> username with the hostname to get the email.
>
> By the way, I suspect this is the answer to "what do people type for
> this prompt". It is probably more about a safety on bad ident than it is
> about people routinely updating the information. I actually think it
> would make more sense to drop the prompt entirely and just die when the
> user has not given us a usable ident.
Yeah, I agree with pretty much everything you said in this thread
(including that environment and config are equally likely to reflect
user's wish and it does not make much sense to treat environment as
more suspicious).
> But maybe people who do one-off format-patches would rather type
> their name in a prompt than set an environment variable and re-run
> the program.
s/one-off format-patches/one-off send-email/. I think dying will
force them to configure their names once (so that later invocations
do not have to stop) while prompting will force them to type their
names every time, so the current behaviour is probably a false
economy. As long as we caution users in the release notes, it
probably is OK to change the command to die.
^ permalink raw reply
* Re: [PATCH 2/2] send-email: add series-cc-cmd option
From: Felipe Contreras @ 2012-11-13 1:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Joe Perches, Ramkumar Ramachandra, git, Jonathan Nieder,
Pascal Obry
In-Reply-To: <7vd2zigwx6.fsf@alter.siamese.dyndns.org>
On Tue, Nov 13, 2012 at 1:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Joe Perches <joe@perches.com> writes:
>
>> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>>> Felipe Contreras wrote:
>>> > cc-cmd is only per-file, and many times receipients get lost without
>>> > seing the full patch series.
>>>
>>> s/seing/seeing
>>>
>>> > [...]
>>>
>>> Looks good otherwise.
>>
>> s/receipients/recipients/ too
>>
>> Practically this is ok but I think it's unnecessary.
>>
>> Output from git format-patch is always in a single
>> directory.
>
> Sorry, but I do not see how the usefulness (or necessity) of this
> new option is connected to the fact that you can tell the command to
> write the patches into a single (possibly new) directory. Care to
> explain?
Basically 'git send-email' would do something like this:
git format-patch -10 -o /tmp
for x in /tmp/*; do cc_cmd $x; done
And the cc-cmd would do something like:
cc_cmd() { do_stuff_with $(dirname $1)/*.patch }
--
Felipe Contreras
^ permalink raw reply
* [PATCH 0/2] remote-bzr: trivial updates
From: Felipe Contreras @ 2012-11-13 1:03 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
Hi,
Here's a fix for older versions of bzr, and a check for local repositories.
Felipe Contreras (2):
remote-bzr: add support for older versions of bzr
remote-bzr: detect local repositories
contrib/remote-helpers/git-remote-bzr | 54 +++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 21 deletions(-)
--
1.8.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox