Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-11 Thread Jeff King
On Sun, Nov 11, 2012 at 09:02:48AM +0200, Kalle Olavi Niemitalo wrote:

> If git did the same thing as cvs here, i.e. ignore the signals in
> the parent process only and check the exit status of the editor,
> I think that would be OK.

Silly me. When I thought through the impact of Paul's patch, I knew that
we would notice signal death of the editor. But I totally forgot to
consider that the blocked signal is inherited by the child process. I
think we just need to move the signal() call to after we've forked. Like
this (on top of Paul's patch):

diff --git a/editor.c b/editor.c
index 3ca361b..0ed23ce 100644
--- a/editor.c
+++ b/editor.c
@@ -38,11 +38,20 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
 
if (strcmp(editor, ":")) {
const char *args[] = { editor, path, NULL };
+   struct child_process p;
int ret;
 
+   memset(&p, 0, sizeof(p));
+   p.argv = args;
+   p.env = env;
+   p.use_shell = 1;
+   if (start_command(&p) < 0)
+   return error("unable to start editor '%s'", editor);
+
sigchain_push(SIGINT, SIG_IGN);
-   ret = run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, 
env);
+   ret = finish_command(&p);
sigchain_pop(SIGINT);
+
if (ret)
return error("There was a problem with the editor 
'%s'.",
editor);

Note that this will give you a slightly verbose message from git.
Potentially we could notice editor death due to SIGINT and suppress the
message, under the assumption that the user hit ^C and does not need to
be told.

-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 (Nov 2012, #02; Fri, 9)

2012-11-11 Thread Andreas Schwab
Kalle Olavi Niemitalo  writes:

> and neither process blocked any signals (not even SIGCHLD as system(3)
> would).

If you don't have a SIGCHLD handler it won't matter anyway.

Andreas.

-- 
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: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Kalle Olavi Niemitalo
Paul Fox  writes:

> you're sending SIGINT to the cvs commit command, and that causes the
> editor to die right away?

That's right.  It is not a quirk of shell-mode in Emacs, because
I get the same result with ^C in xterm too.

% EDITOR="$HOME/prefix/x86_64-unknown-linux-gnu/bin/emacsclient --current-frame"
% export EDITOR
% cvs commit BUGIT
Waiting for Emacs...^Ccvs commit: warning: editor session failed

Log message unchanged or not specified
a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
Action: (continue) a
cvs [commit aborted]: aborted by user
% 

While cvs was waiting from emacsclient:

% cat /proc/2030/stat
2030 (cvs) S 1849 2030 1849 34816 2030 4202496 598 0 0 0 0 0 0 0 20 0 1 0 
94752537 34254848 410 18446744073709551615 140168182550528 140168183348316 
140737407935424 140737407931680 140168163193950 0 0 6 20513 0 0 0 17 2 0 0 0 0 0
% grep 'Name\|Pid\|Sig' /proc/2030/status
Name:   cvs
Pid:2030
PPid:   1849
TracerPid:  0
SigQ:   0/28998
SigPnd: 
SigBlk: 
SigIgn: 0006
SigCgt: 000180005021
% cat /proc/2031/stat
2031 (emacsclient) S 2030 2030 1849 34816 2030 4202496 155 0 0 0 0 0 0 0 20 0 1 
0 94752538 4169728 81 18446744073709551615 4194304 4210620 140735996104016 
140735996095456 140664960886018 0 0 0 0 0 0 0 17 1 0 0 0 0 0
% grep 'Name\|Pid\|Sig' /proc/2031/status
Name:   emacsclient
Pid:2031
PPid:   2030
TracerPid:  0
SigQ:   0/28998
SigPnd: 
SigBlk: 
SigIgn: 
SigCgt: 
%

which I interpret to mean both processes were in process group
2030, the cvs process ignored SIGINT and SIGQUIT, the emacsclient
process neither ignored nor handled any signals, and neither
process blocked any signals (not even SIGCHLD as system(3) would).
When ^C in the terminal sent SIGINT to the process group, it
terminated the emacsclient process only.

If git did the same thing as cvs here, i.e. ignore the signals in
the parent process only and check the exit status of the editor,
I think that would be OK.
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Paul Fox
kalle olavi niemitalo wrote:
 > Paul Fox  writes:
 > 
 > > when i implemented the change, i wondered if some twisted emacs
 > > workflow would be an issue. ;-)  and i almost blocked SIGQUIT as
 > > well -- the two programs i looked at for precedent (CVS and MH) both
 > > block both SIGQUIT and SIGINT when spawning an editor.
 > >
 > > but since emacs users must have dealt with CVS for a long time before
 > > dealing with git, how might they have done so?
 > 
 > I think I usually ran CVS via vc.el, which prompts for a commit
 > message in Emacs before it runs cvs commit.  So CVS did not need
 > to run $EDITOR.
 > 
 > I just tried emacsclient with CVS 1.12.13-MirDebian-9, and it
 > behaves somewhat differently from Git with pf/editor-ignore-sigint.
 > When I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:
 > 
 > cvs commit: warning: editor session failed
 > 
 > Log message unchanged or not specified
 > a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
 > Action: (continue) 

you're sending SIGINT to the cvs commit command, and that causes the
editor to die right away?  that's surprising.  i can replicate your
described behavior by setting $VISUAL to a script that just sleeps, and
sending SIGTERM to the cvs commit process.  but not by sending SIGINT.

well, i'm not sure what to say.  there's a real problem when using the
current code and traditional editors.  i thought that the patch in
pf/editor-ignore-sigint reflected standard practice, and indeed it
accomplishes exactly the right thing with those editors.  you've shown
a particular work flow involving emacsclient that won't work anymore
with the change made, though there are workarounds.  perhaps there's
something the other editors themselves should be doing differently,
but i don't know what that might be.

paul

 > 
 > and then I can choose to abort.
 > 
 > With strace, it looks like CVS sets SIG_IGN as the handler of
 > SIGINT and SIGQUIT only in the parent process after forking, not
 > in the child process that executes the editor.
 > 
 > CVS also temporarily blocks signals by calling sigprocmask, but
 > it undoes that before it forks or waits for the child process.

=-
 paul fox, p...@foxharp.boston.ma.us (arlington, ma, where it's 45.5 degrees)
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Andreas Schwab
Kalle Olavi Niemitalo  writes:

> With strace, it looks like CVS sets SIG_IGN as the handler of
> SIGINT and SIGQUIT only in the parent process after forking, not
> in the child process that executes the editor.
>
> CVS also temporarily blocks signals by calling sigprocmask, but
> it undoes that before it forks or waits for the child process.

This emulates what system(3) does.

Andreas.

-- 
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: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Junio C Hamano
SZEDER Gábor  writes:

>> I think it is better to keep the tests simple and maintainable.
>
> Maintainable?  There is nothing to maintain here
> ...
> OTOH, this series has some serious drawbacks.
>
> It makes debugging more difficult

Are these referring to the same aspect of the series?  The concern
you described about debuggability matches my impression IIRC back
when I took a look at the series, which I would count as a large
part of keeping tests maintainable.

But you may be referring to something different (sorry, not on my
primary machine yet).
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Kalle Olavi Niemitalo
Paul Fox  writes:

> when i implemented the change, i wondered if some twisted emacs
> workflow would be an issue. ;-)  and i almost blocked SIGQUIT as
> well -- the two programs i looked at for precedent (CVS and MH) both
> block both SIGQUIT and SIGINT when spawning an editor.
>
> but since emacs users must have dealt with CVS for a long time before
> dealing with git, how might they have done so?

I think I usually ran CVS via vc.el, which prompts for a commit
message in Emacs before it runs cvs commit.  So CVS did not need
to run $EDITOR.

I just tried emacsclient with CVS 1.12.13-MirDebian-9, and it
behaves somewhat differently from Git with pf/editor-ignore-sigint.
When I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:

cvs commit: warning: editor session failed

Log message unchanged or not specified
a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs
Action: (continue) 

and then I can choose to abort.

With strace, it looks like CVS sets SIG_IGN as the handler of
SIGINT and SIGQUIT only in the parent process after forking, not
in the child process that executes the editor.

CVS also temporarily blocks signals by calling sigprocmask, but
it undoes that before it forks or waits for the child process.
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Felipe Contreras
On Sat, Nov 10, 2012 at 1:32 PM, SZEDER Gábor  wrote:
> Hi,
>
> On Fri, Nov 09, 2012 at 07:33:31PM -0500, Jeff King wrote:
>> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:
>> > > * fc/completion-test-simplification (2012-10-29) 2 commits
>> > >  - completion: simplify __gitcomp test helper
>> > >  - completion: refactor __gitcomp related tests
>> > >
>> > >  Clean up completion tests.
>> > >
>> > >  There were some comments on the list.
>> > >
>> > >  Expecting a re-roll.
>> >
>> > The second patch I can re-roll, but the first patch needs some
>> > external input. My preference is that tests should also be simple and
>> > maintainable, SZEDER's preference is that tests are better being
>> > explicit and verbose (even if harder to maintain) to minimize possible
>> > issues in the tests.
>>
>> I think it is better to keep the tests simple and maintainable.
>
> Maintainable?  There is nothing to maintain here.  Users' completion
> scripts depend on __gitcomp(), so its behavior shouldn't be changed.
> It can only be extended by a fifth parameter or by quoting words when
> necessary, but these future changes must not alter the current
> behavior checked by these tests, therefore even then these tests must
> be left intact.

I disagree. If we add a new parameter to __gitcomp(), and we need to
add a new parameter to test_gitcomp(), so be it. Yes, we might change
the behavior of the other tests, but that's what reviews are for: to
make sure we don't alter other behavior by mistake. That's what we do
for code, and that what we should do for tests.

But in this particular case nothing would need to change because
test_gitcomp() would pass whatever arguments it receives, being four,
or five, or twenty. So this is not a concern, maybe some other kind of
change, but not this.

Compare this:
http://article.gmane.org/gmane.comp.version-control.git/208168

To this:
http://article.gmane.org/gmane.comp.version-control.git/207927

If we ever need to make changes to the __gitcomp tests, a small change
is better than a big change; IOW: the test code would be more
maintainable.

Even more, the end result is much less code: less code = more maintainability.

> Simple?  Currently you only need to look at __gitcomp() and the test
> itself to understand what's going on.  With this series you'll also
> need to look at test_gitcomp(), figure out what its parameters are
> supposed to mean, and possibly get puzzled on the way why __gitcomp()
> is now seemingly called with only one parameter.

Maybe it's easier for you to understand, but certainly not for other
people: 'declare -a COMPREPLY'? cur? print_comp? What does that even
mean? Chances are most people don't even know what __gitcomp is.
Fortunately they don't have to dig too deep to find all those things
out, but they can do the same for test_gitcomp().

It might make sense to add some comment on top of test_gitcomp to
explain the arguments and the input to make things easier, but that
would only make it more readable, not less, than the current
situation, because right now you would have to add a similar comment
to each and every block of code that calls __gitcomp.

Functions make our life easier.

> So, I don't see much benefit in this series (except the part to use
> print_comp instead of "change IFS && echo", but that's already done in
> this patch:
> http://article.gmane.org/gmane.comp.version-control.git/207927).

Yes, and that version is much bigger than this:
http://article.gmane.org/gmane.comp.version-control.git/208168

Code that allows the later patch is more maintainable.

> OTOH, this series has some serious drawbacks.
>
> It makes debugging more difficult.  While working on the quoting
> issues I managed to break completion tests many-many times lately.  In
> normal tests I could add a few debugging instructions to the failed
> test to find out where the breakage lies, without affecting other
> tests.  However, if the failed test uses the test_completion() helper,
> then I have to add debugging instructions to test_completion() itself,
> too.  This is bad, because many tests use this helper function and are
> therefore affected by the debugging instructions, producing truckloads
> of output making it difficult to dig out the relevant parts, or, worse
> yet, causing breakages in other tests.  With this series the same
> difficulties will come to __gitcomp() tests, too.

What I do is copy the function to test_gitcomp2() and add the
debugging there, and only call it from the places where I want the
debugging. I don't think this is an issue.

In fact, I think it's an advantage; sometimes that's exactly what you
want, to add the debugging for everything.

> It can also encourage writing bad tests, similar to those that managed
> to cram many test_completion() lines into a single tests, giving me
> headaches to figure out what went wrong this time.

That's policy. We can decide what each test-case contains right here,
on the mailing list. There's no nee

Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Junio C Hamano
Jeff King  writes:

> On Fri, Nov 09, 2012 at 12:27:35PM -0800, Junio C Hamano wrote:
>
>> What we should have arranged was to have https://github.com/git/git
>> (which is not even owned by me, but I asked somebody at GitHub to
>> assign me a write privilege) writable by the interim maintainer, so
>> that normal people would keep pulling from there, while the interim
>> maintainer can choose to publish broken-out branches to his
>> repository.
>
> Yes, I have write access to that repository, too, but I intentionally
> held off from updating it out of a sense of nervousness. I figured if I
> screwed up anything too badly, people who were clued-in enough to switch
> to pulling from my repository would be clued-in enough to rebase across
> any too-horrible mistake I made. ;)

That "nervousness" reminds me of myself when I took over.  Before I
could ask for a few weeks of practice period, Linus arranged to have
folks at k.org to chown the authoritative location to me, declaring
"no practice period; it's already done and it's all yours".

And I made at least one mistake pushing 'master' with one commit
rewound too much (corrected by pushing an extra merge).  Luckily,
the world did not end ;-).

> I think if we do this again, I will make the same split you do (git/git
> for integration branches, peff/git as a mirror of my private repo).

I am fairly sure I'll have to ask you (or somebody else) again next
year around late September.

>> And it is not too late to do so; from the look of your "What's
>> cooking", you are doing pretty well ;-).
>
> Any fool can merge topics to master. The real test will be how many
> regressions people report in the next two weeks. :)

I agree that the actual merging to 'master' is mechanical with the
procedure built around Meta/Reintegrate.  Important decisions are
made before you merge a topic to 'next' and mark topics as "Will
merge to 'master'."  My comment was about that, and your responses
to the list messages.

> By the way, I did not touch 'maint' at all while you were gone. I don't
> know what your usual method is for keeping track of maint-worthy topics
> after they have gone to master. The usual "what's cooking" workflow
> keeps track of things going to master, but no more; I'd guess you
> probably just merge to maint when you delete them from last cycle's
> "graduated to master" list.

That is done by eyeballing output from Meta/GRADUATED (which spits
out something that could be fed to shell, but I do not fully trust
its logic, and always eyeball them before I prepare the temporary
file to feed Meta/Reintegrate to update 'maint').
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread Paul Fox
kalle olavi niemitalo wrote:
 > Jeff King  writes:
 > 
 > >  Comments welcome from people using unusual editors (e.g., a script that
 > >  starts an editor in another window then blocks, waiting for the user to
 > >  finish).
 > 
 > I often run a shell in Emacs in X, then start git commit in that
 > shell.  $EDITOR is emacsclient --current-frame, which asks the
 > existing Emacs instance to load the file and waits until I press
 > C-x # in Emacs to mark the file done.  If I want to abort the
 > commit, it is most intuitive to return to the *Shell* buffer in
 > Emacs and press C-c C-c (comint-interrupt-subjob) to send SIGINT
 > to git from there.  (I see that "an empty message aborts the
 > commit", and indeed it does, but well, I prefer not to trust such
 > a feature if I can instead just interrupt the thing.)
 > 
 > With pf/editor-ignore-sigint, C-c C-c in the *Shell* buffer kills
 > neither git nor the emacsclient started by git.  This is not good.
 > SIGQUIT from C-c C-\ (comint-quit-subjob) still works though.

when i implemented the change, i wondered if some twisted emacs
workflow would be an issue. ;-)  and i almost blocked SIGQUIT as
well -- the two programs i looked at for precedent (CVS and MH) both
block both SIGQUIT and SIGINT when spawning an editor.

but since emacs users must have dealt with CVS for a long time before
dealing with git, how might they have done so?

the existing git behavior is bad for non-emacs users, and git itself
provides an abort-the-operation mechanism (i.e., writing an empty
message), so i'm not convinced your use case invalidates the new
behavior.  (though it might spotlight a need for this being prominent
in release notes.)

paul
=-
 paul fox, p...@foxharp.boston.ma.us (arlington, ma, where it's 40.6 degrees)
--
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 (Nov 2012, #02; Fri, 9)

2012-11-10 Thread SZEDER Gábor
Hi,

On Fri, Nov 09, 2012 at 07:33:31PM -0500, Jeff King wrote:
> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:
> > > * fc/completion-test-simplification (2012-10-29) 2 commits
> > >  - completion: simplify __gitcomp test helper
> > >  - completion: refactor __gitcomp related tests
> > >
> > >  Clean up completion tests.
> > >
> > >  There were some comments on the list.
> > >
> > >  Expecting a re-roll.
> > 
> > The second patch I can re-roll, but the first patch needs some
> > external input. My preference is that tests should also be simple and
> > maintainable, SZEDER's preference is that tests are better being
> > explicit and verbose (even if harder to maintain) to minimize possible
> > issues in the tests.
> 
> I think it is better to keep the tests simple and maintainable.

Maintainable?  There is nothing to maintain here.  Users' completion
scripts depend on __gitcomp(), so its behavior shouldn't be changed.
It can only be extended by a fifth parameter or by quoting words when
necessary, but these future changes must not alter the current
behavior checked by these tests, therefore even then these tests must
be left intact.

Simple?  Currently you only need to look at __gitcomp() and the test
itself to understand what's going on.  With this series you'll also
need to look at test_gitcomp(), figure out what its parameters are
supposed to mean, and possibly get puzzled on the way why __gitcomp()
is now seemingly called with only one parameter.

So, I don't see much benefit in this series (except the part to use
print_comp instead of "change IFS && echo", but that's already done in
this patch:
http://article.gmane.org/gmane.comp.version-control.git/207927).

OTOH, this series has some serious drawbacks.

It makes debugging more difficult.  While working on the quoting
issues I managed to break completion tests many-many times lately.  In
normal tests I could add a few debugging instructions to the failed
test to find out where the breakage lies, without affecting other
tests.  However, if the failed test uses the test_completion() helper,
then I have to add debugging instructions to test_completion() itself,
too.  This is bad, because many tests use this helper function and are
therefore affected by the debugging instructions, producing truckloads
of output making it difficult to dig out the relevant parts, or, worse
yet, causing breakages in other tests.  With this series the same
difficulties will come to __gitcomp() tests, too.

It can also encourage writing bad tests, similar to those that managed
to cram many test_completion() lines into a single tests, giving me
headaches to figure out what went wrong this time.


Best,
Gábor

--
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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Felipe Contreras
On Sat, Nov 10, 2012 at 1:33 AM, Jeff King  wrote:
> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:
>
>> > * fc/fast-export-fixes (2012-11-08) 14 commits
>> >  - fast-export: don't handle uninteresting refs
>> >  - fast-export: make sure updated refs get updated
>> >  - fast-export: fix comparison in tests
>> >  - fast-export: trivial cleanup
>> >  - remote-testgit: make clear the 'done' feature
>> >  - remote-testgit: report success after an import
>> >  - remote-testgit: exercise more features
>> >  - remote-testgit: cleanup tests
>> >  - remote-testgit: remove irrelevant test
>> >  - remote-testgit: get rid of non-local functionality
>> >  - Add new simplified git-remote-testgit
>> >  - Rename git-remote-testgit to git-remote-testpy
>> >  - remote-testgit: fix direction of marks
>> >  - fast-export: avoid importing blob marks
>> >
>> >  Improvements to fix fast-export bugs, including how refs pointing to
>> >  already-seen commits are handled. An earlier 4-commit version of this
>> >  series looked good to me, but this much-expanded version has not seen
>> >  any comments.
>> >
>> >  Needs review.
>>
>> I can send the previous 4-commit version if needed, the only thing
>> that changed is the commit messages.
>
> In the actual code, perhaps, but aren't there significant changes to the
> git-remote-testgit infrastructure that were not originally present? That
> could use some review.
>
> I also seem to recall that the tests in this version rely on the presence of 
> bash;
> don't we still need to mark the tests with a prerequisite?

I meant in the 4-commits.

>> > * fc/completion-test-simplification (2012-10-29) 2 commits
>> >  - completion: simplify __gitcomp test helper
>> >  - completion: refactor __gitcomp related tests
>> >
>> >  Clean up completion tests.
>> >
>> >  There were some comments on the list.
>> >
>> >  Expecting a re-roll.
>>
>> The second patch I can re-roll, but the first patch needs some
>> external input. My preference is that tests should also be simple and
>> maintainable, SZEDER's preference is that tests are better being
>> explicit and verbose (even if harder to maintain) to minimize possible
>> issues in the tests.
>
> I think it is better to keep the tests simple and maintainable. If there
> are multiple ways to do things and they all need testing, then that
> should be clear from the tests, not done haphazardly because some tests
> happen to use a different way of doing things.

Good, that's what my first patch does; no functional changes, just
refactor code into a single function.

> I seem to recall there was a one-liner fix that needed to be rolled in,
> which is why I held it out of next.

Yes, that I can reroll.

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: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Jeff King
On Fri, Nov 09, 2012 at 12:27:35PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I have not been pushing the individual topic branches to make life
> > easier for people who usually just track Junio's kernel.org repository,
> > and would not welcome suddenly getting a hundred extra remote branches.
> > I can make them public if it makes life easier for people, but it may
> > not be worth it at this point, with Junio returning soon.
> 
> What we should have arranged was to have https://github.com/git/git
> (which is not even owned by me, but I asked somebody at GitHub to
> assign me a write privilege) writable by the interim maintainer, so
> that normal people would keep pulling from there, while the interim
> maintainer can choose to publish broken-out branches to his
> repository.

Yes, I have write access to that repository, too, but I intentionally
held off from updating it out of a sense of nervousness. I figured if I
screwed up anything too badly, people who were clued-in enough to switch
to pulling from my repository would be clued-in enough to rebase across
any too-horrible mistake I made. ;)

I think if we do this again, I will make the same split you do (git/git
for integration branches, peff/git as a mirror of my private repo).

> And it is not too late to do so; from the look of your "What's
> cooking", you are doing pretty well ;-).

Any fool can merge topics to master. The real test will be how many
regressions people report in the next two weeks. :)

By the way, I did not touch 'maint' at all while you were gone. I don't
know what your usual method is for keeping track of maint-worthy topics
after they have gone to master. The usual "what's cooking" workflow
keeps track of things going to master, but no more; I'd guess you
probably just merge to maint when you delete them from last cycle's
"graduated to master" list.

I just let them stew in master for a bit longer, and we can easily find
and merge them with "git branch --no-merged maint | grep maint".

-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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Jeff King
On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:

> > * fc/fast-export-fixes (2012-11-08) 14 commits
> >  - fast-export: don't handle uninteresting refs
> >  - fast-export: make sure updated refs get updated
> >  - fast-export: fix comparison in tests
> >  - fast-export: trivial cleanup
> >  - remote-testgit: make clear the 'done' feature
> >  - remote-testgit: report success after an import
> >  - remote-testgit: exercise more features
> >  - remote-testgit: cleanup tests
> >  - remote-testgit: remove irrelevant test
> >  - remote-testgit: get rid of non-local functionality
> >  - Add new simplified git-remote-testgit
> >  - Rename git-remote-testgit to git-remote-testpy
> >  - remote-testgit: fix direction of marks
> >  - fast-export: avoid importing blob marks
> >
> >  Improvements to fix fast-export bugs, including how refs pointing to
> >  already-seen commits are handled. An earlier 4-commit version of this
> >  series looked good to me, but this much-expanded version has not seen
> >  any comments.
> >
> >  Needs review.
> 
> I can send the previous 4-commit version if needed, the only thing
> that changed is the commit messages.

In the actual code, perhaps, but aren't there significant changes to the
git-remote-testgit infrastructure that were not originally present? That
could use some review.

I also seem to recall that the tests in this version rely on the presence of 
bash;
don't we still need to mark the tests with a prerequisite?

> > * fc/completion-test-simplification (2012-10-29) 2 commits
> >  - completion: simplify __gitcomp test helper
> >  - completion: refactor __gitcomp related tests
> >
> >  Clean up completion tests.
> >
> >  There were some comments on the list.
> >
> >  Expecting a re-roll.
> 
> The second patch I can re-roll, but the first patch needs some
> external input. My preference is that tests should also be simple and
> maintainable, SZEDER's preference is that tests are better being
> explicit and verbose (even if harder to maintain) to minimize possible
> issues in the tests.

I think it is better to keep the tests simple and maintainable. If there
are multiple ways to do things and they all need testing, then that
should be clear from the tests, not done haphazardly because some tests
happen to use a different way of doing things.

I seem to recall there was a one-liner fix that needed to be rolled in,
which is why I held it out of next.

> > * fc/remote-bzr (2012-11-08) 5 commits
> >  - remote-bzr: update working tree
> >  - remote-bzr: add support for remote repositories
> >  - remote-bzr: add support for pushing
> >  - remote-bzr: add simple tests
> >  - Add new remote-bzr transport helper
> >
> >  New remote helper for bzr.
> >
> >  Will merge to 'next'.
> 
> I already have a newer version of this with support for special modes:
> executable files, symlinks, etc. I think a reroll would make sense.

Thanks for letting me know.

-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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Felipe Contreras
Hi,

On Fri, Nov 9, 2012 at 8:23 PM, Jeff King  wrote:

> * fc/fast-export-fixes (2012-11-08) 14 commits
>  - fast-export: don't handle uninteresting refs
>  - fast-export: make sure updated refs get updated
>  - fast-export: fix comparison in tests
>  - fast-export: trivial cleanup
>  - remote-testgit: make clear the 'done' feature
>  - remote-testgit: report success after an import
>  - remote-testgit: exercise more features
>  - remote-testgit: cleanup tests
>  - remote-testgit: remove irrelevant test
>  - remote-testgit: get rid of non-local functionality
>  - Add new simplified git-remote-testgit
>  - Rename git-remote-testgit to git-remote-testpy
>  - remote-testgit: fix direction of marks
>  - fast-export: avoid importing blob marks
>
>  Improvements to fix fast-export bugs, including how refs pointing to
>  already-seen commits are handled. An earlier 4-commit version of this
>  series looked good to me, but this much-expanded version has not seen
>  any comments.
>
>  Needs review.

I can send the previous 4-commit version if needed, the only thing
that changed is the commit messages.

I think it's unfortunate that 4-commit version would not be mentioning
that it fixes the above tests, but hey; I did what I could.

> * fc/zsh-completion (2012-10-29) 3 commits
>  - completion: add new zsh completion
>  - completion: add new __gitcompadd helper
>  - completion: get rid of empty COMPREPLY assignments
>
>  There were some comments on this, but I wasn't clear on the outcome.
>
>  Need to take a closer look.

SZEDER should probably take a look. I think it should be better than
the previous series.

> * fc/completion-test-simplification (2012-10-29) 2 commits
>  - completion: simplify __gitcomp test helper
>  - completion: refactor __gitcomp related tests
>
>  Clean up completion tests.
>
>  There were some comments on the list.
>
>  Expecting a re-roll.

The second patch I can re-roll, but the first patch needs some
external input. My preference is that tests should also be simple and
maintainable, SZEDER's preference is that tests are better being
explicit and verbose (even if harder to maintain) to minimize possible
issues in the tests.

> * fc/remote-testgit-feature-done (2012-10-29) 1 commit
>  - remote-testgit: properly check for errors
>
>  Needs review.

Sverre probably should reply. I think I already addressed his comments
and the patch should be OK to push.

But probably it's not that important considering the testgit
refactoring, and also I'm thinking that we need to actually check the
status of the process[1] because the situation is still not OK with
pushing, and I'm learning it the hard way with a buggy remote helper.

> * fc/remote-bzr (2012-11-08) 5 commits
>  - remote-bzr: update working tree
>  - remote-bzr: add support for remote repositories
>  - remote-bzr: add support for pushing
>  - remote-bzr: add simple tests
>  - Add new remote-bzr transport helper
>
>  New remote helper for bzr.
>
>  Will merge to 'next'.

I already have a newer version of this with support for special modes:
executable files, symlinks, etc. I think a reroll would make sense.

> * fc/remote-hg (2012-11-04) 16 commits
>  - remote-hg: the author email can be null
>  - remote-hg: add option to not track branches
>  - remote-hg: add extra author test
>  - remote-hg: add tests to compare with hg-git
>  - remote-hg: add bidirectional tests
>  - test-lib: avoid full path to store test results
>  - remote-hg: add basic tests
>  - remote-hg: fake bookmark when there's none
>  - remote-hg: add compat for hg-git author fixes
>  - remote-hg: add support for hg-git compat mode
>  - remote-hg: match hg merge behavior
>  - remote-hg: make sure the encoding is correct
>  - remote-hg: add support to push URLs
>  - remote-hg: add support for remote pushing
>  - remote-hg: add support for pushing
>  - Add new remote-hg transport helper
>
>  New remote helper for hg.
>
>  Will merge to 'next'.

:)

I have a few patches on top of this, but they can probably wait.

Cheers.

[1] http://article.gmane.org/gmane.comp.version-control.git/208139

-- 
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: What's cooking in git.git (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Kalle Olavi Niemitalo
Jeff King  writes:

>  Comments welcome from people using unusual editors (e.g., a script that
>  starts an editor in another window then blocks, waiting for the user to
>  finish).

I often run a shell in Emacs in X, then start git commit in that
shell.  $EDITOR is emacsclient --current-frame, which asks the
existing Emacs instance to load the file and waits until I press
C-x # in Emacs to mark the file done.  If I want to abort the
commit, it is most intuitive to return to the *Shell* buffer in
Emacs and press C-c C-c (comint-interrupt-subjob) to send SIGINT
to git from there.  (I see that "an empty message aborts the
commit", and indeed it does, but well, I prefer not to trust such
a feature if I can instead just interrupt the thing.)

With pf/editor-ignore-sigint, C-c C-c in the *Shell* buffer kills
neither git nor the emacsclient started by git.  This is not good.
SIGQUIT from C-c C-\ (comint-quit-subjob) still works though.
--
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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Junio C Hamano
Jeff King  writes:

> I have not been pushing the individual topic branches to make life
> easier for people who usually just track Junio's kernel.org repository,
> and would not welcome suddenly getting a hundred extra remote branches.
> I can make them public if it makes life easier for people, but it may
> not be worth it at this point, with Junio returning soon.

What we should have arranged was to have https://github.com/git/git
(which is not even owned by me, but I asked somebody at GitHub to
assign me a write privilege) writable by the interim maintainer, so
that normal people would keep pulling from there, while the interim
maintainer can choose to publish broken-out branches to his
repository.

And it is not too late to do so; from the look of your "What's
cooking", you are doing pretty well ;-).
--
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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Jeff King
On Fri, Nov 09, 2012 at 09:06:47PM +0100, Ralf Thielow wrote:

> > It seems that the repo doesn't contain the integration branches?!?
> >
> > $ git remote add peff git://github.com/peff/git.git
> > $ git fetch -v peff
> > From git://github.com/peff/git
> >  * [new branch]  maint  -> peff/maint
> >  * [new branch]  master -> peff/master
> >  * [new branch]  next   -> peff/next
> >  * [new branch]  pu -> peff/pu
> >  * [new branch]  todo   -> peff/todo
> > $
> 
> But "integration branches" means "master", "next" and "pu" than I haven't
> said anything. ;) Sorry for the noise.

Right. :)

I have not been pushing the individual topic branches to make life
easier for people who usually just track Junio's kernel.org repository,
and would not welcome suddenly getting a hundred extra remote branches.
I can make them public if it makes life easier for people, but it may
not be worth it at this point, with Junio returning soon.

-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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Ralf Thielow
On Fri, Nov 9, 2012 at 9:01 PM, Ralf Thielow  wrote:
> On Fri, Nov 9, 2012 at 8:23 PM, Jeff King  wrote:
>>
>> You can find the changes described here in the integration branches of
>> my repository at:
>>
>>   git://github.com/peff/git.git
>
> It seems that the repo doesn't contain the integration branches?!?
>
> $ git remote add peff git://github.com/peff/git.git
> $ git fetch -v peff
> From git://github.com/peff/git
>  * [new branch]  maint  -> peff/maint
>  * [new branch]  master -> peff/master
>  * [new branch]  next   -> peff/next
>  * [new branch]  pu -> peff/pu
>  * [new branch]  todo   -> peff/todo
> $

But "integration branches" means "master", "next" and "pu" than I haven't
said anything. ;) Sorry for the noise.
--
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 (Nov 2012, #02; Fri, 9)

2012-11-09 Thread Ralf Thielow
On Fri, Nov 9, 2012 at 8:23 PM, Jeff King  wrote:
>
> You can find the changes described here in the integration branches of
> my repository at:
>
>   git://github.com/peff/git.git

It seems that the repo doesn't contain the integration branches?!?

$ git remote add peff git://github.com/peff/git.git
$ git fetch -v peff
>From git://github.com/peff/git
 * [new branch]  maint  -> peff/maint
 * [new branch]  master -> peff/master
 * [new branch]  next   -> peff/next
 * [new branch]  pu -> peff/pu
 * [new branch]  todo   -> peff/todo
$
--
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