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