* Re: Storing additional information in commit headers
From: martin f krafft @ 2011-08-01 20:51 UTC (permalink / raw)
To: Martin Langhoff, git discussion list, Petr Baudis,
Clemens Buchacher
In-Reply-To: <CACPiFCLPgsC+9cX7r33oCQ2AnuRXMTqOAE5RZLS7hXdHc6B-9Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]
also sprach Martin Langhoff <martin.langhoff@gmail.com> [2011.08.01.2133 +0200]:
> What data are you trying to include? Some time ago, I had similar
> ideas to yours for a while... and it ended up being that all I needed
> was to put the additional data /in a file/ and commit that file.
Hi, thanks for taking the time to reply to me!
I am trying to store the top-base of a TopGit branch, which is the
merge of all a branch's dependencies.
TopGit uses refs for that, but a ref can only ever point at one such
merge, and so it's hard-to-impossible to reconstruct a branch
dependency in the past.
TopGit does use files in the worktree too. I would love to get rid
of this as well, since a file like .topmsg (which differs between
all branches, even related ones), requires to always remember to use
the 'ours' merge driver, which requires setup, which makes it harder
to use.
> If you are using a wrapper program,
I am trying to stay as close as possible to plain Git. All of this
could easily be done by a wrapper, but a wrapper always makes too
many assumptions to become a viable standard for Debian packaging.
> it's valid/sane, in the preparations to commit, perhaps ensuring
> that a pre-commit-hook script is in place and executable.
Again, that requires setup, which increases the barrier of entry to
passerby's and new contributors.
--
martin | http://madduck.net/ | http://two.sentenc.es/
"verbing weirds language."
-- calvin
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/sig-policy/999bbcc4/current) --]
[-- Type: application/pgp-signature, Size: 1124 bytes --]
^ permalink raw reply
* [PATCH v2] Documentation/submodule: add command references and update options
From: Jens Lehmann @ 2011-08-01 20:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Marc Branchaud, Nikolai Weibull
Reference the "git diff" and "git status" commands where they learned
functionality that in earlier git versions was only available through the
'summary' and 'status' subcommands of "git submodule".
The short option '-n' for '--summary-limit' was missing from the synopsis
and the --init option was missing from the "options" section, add those
there. And while at it, quote all options in backticks so they are
decorated properly in the output formats which support that.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
In this version the options are quoted in backticks and the phrase "will
provide the same information" has been changed to "provide that information
too" to inform the user that it will provide "the same and a lot of other"
information.
Documentation/git-submodule.txt | 38 ++++++++++++++++++++++++++------------
1 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 0ec8574..67cf5f0 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,7 +15,8 @@ SYNOPSIS
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
[--reference <repository>] [--merge] [--recursive] [--] [<path>...]
-'git submodule' [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
+'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
+ [commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--] [<path>...]
@@ -108,8 +109,13 @@ status::
repository and `U` if the submodule has merge conflicts.
This command is the default command for 'git submodule'.
+
-If '--recursive' is specified, this command will recurse into nested
+If `--recursive` is specified, this command will recurse into nested
submodules, and show their status as well.
++
+If you are only interested in changes of the currently initialized
+submodules with respect to the commit recorded in the index or the HEAD,
+linkgit:git-status[1] and linkgit:git-diff[1] will provide that information
+too (and can also report changes to a submodule's work tree).
init::
Initialize the submodules, i.e. register each submodule name
@@ -125,26 +131,29 @@ init::
update::
Update the registered submodules, i.e. clone missing submodules and
checkout the commit specified in the index of the containing repository.
- This will make the submodules HEAD be detached unless '--rebase' or
- '--merge' is specified or the key `submodule.$name.update` is set to
+ This will make the submodules HEAD be detached unless `--rebase` or
+ `--merge` is specified or the key `submodule.$name.update` is set to
`rebase` or `merge`.
+
If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
-submodule with the --init option.
+submodule with the `--init` option.
+
-If '--recursive' is specified, this command will recurse into the
+If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
summary::
Show commit summary between the given commit (defaults to HEAD) and
working tree/index. For a submodule in question, a series of commits
in the submodule between the given super project commit and the
- index or working tree (switched by --cached) are shown. If the option
- --files is given, show the series of commits in the submodule between
+ index or working tree (switched by `--cached`) are shown. If the option
+ `--files` is given, show the series of commits in the submodule between
the index of the super project and the working tree of the submodule
- (this option doesn't allow to use the --cached option or to provide an
+ (this option doesn't allow to use the `--cached` option or to provide an
explicit commit).
++
+Using the `--submodule=log` option with linkgit:git-diff[1] will provide that
+information too.
foreach::
Evaluates an arbitrary shell command in each checked out submodule.
@@ -155,9 +164,9 @@ foreach::
superproject, $sha1 is the commit as recorded in the superproject,
and $toplevel is the absolute path to the top-level of the superproject.
Any submodules defined in the superproject but not checked out are
- ignored by this command. Unless given --quiet, foreach prints the name
+ ignored by this command. Unless given `--quiet`, foreach prints the name
of each submodule before evaluating the command.
- If --recursive is given, submodules are traversed recursively (i.e.
+ If `--recursive` is given, submodules are traversed recursively (i.e.
the given shell command is evaluated in nested submodules as well).
A non-zero return from the command in any submodule causes
the processing to terminate. This can be overridden by adding '|| :'
@@ -237,13 +246,18 @@ OPTIONS
If the key `submodule.$name.update` is set to `rebase`, this option is
implicit.
+--init::
+ This option is only valid for the update command.
+ Initialize all submodules for which "git submodule init" has not been
+ called so far before updating.
+
--reference <repository>::
This option is only valid for add and update commands. These
commands sometimes need to clone a remote repository. In this case,
this option will be passed to the linkgit:git-clone[1] command.
+
*NOTE*: Do *not* use this option unless you have read the note
-for linkgit:git-clone[1]'s --reference and --shared options carefully.
+for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
--recursive::
This option is only valid for foreach, update and status commands.
--
1.7.6.351.gb35ac
^ permalink raw reply related
* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: René Scharfe @ 2011-08-01 20:48 UTC (permalink / raw)
To: Jeff King; +Cc: Erik Faye-Lund, Junio C Hamano, git, j6t
In-Reply-To: <20110801182516.GC10302@sigill.intra.peff.net>
Am 01.08.2011 20:25, schrieb Jeff King:
> On Mon, Aug 01, 2011 at 08:02:22PM +0200, Erik Faye-Lund wrote:
>
>>> Weird. What does j.tgz end up looking like? Is it empty, or does it have
>>> bogus data in it? Does gzip actually get invoked at all? Try running
>>> with GIT_TRACE=1. I don't suppose you guys have something like strace,
>>> which might be helpful.
>>>
>>
>> It does have data, and gzip does get invoked:
>> $ gunzip -c j.tgz | wc -c
>>
>> gzip: j.tgz: invalid compressed data--format violated
>> 131072
>>
>> So it seems there are around 130k of valid data before it barfs.
>>
>> Hmm, but when I try the same after re-running the test, I get a
>> different amount of valid data (491520 bytes this time)... Is this a
>> timing-related issue?
>
> Hmm. Non-deterministic output is not good. Could be timing, or we could
> be touching memory we're not supposed to. I just ran t5000 through
> valgrind, and nothing turned up. And of course I can't replicate the
> test failure on Linux. So I assume it really is Windows-specific.
>
> That makes me suspect some difference in how the run-command API works
> on the two platforms, since that is the code here that will be vastly
> difference. I notice the start_command code uses dup() on Windows
> instead of dup2() (I guess you guys don't have dup2?). I wonder if there
> could be some issue with another descriptor accidentally pointing to the
> same spot. But that's just a wild guess.
>
> I think at this point, I'd probably start stepping through the archive
> code with gdb.
Hrm, here is another data point, for what it's worth (Windows 7 x64,
MinGW):
$ git archive v1.7.6 | gzip -cn | md5sum
a0ca1c873a533a5fcd41d248fb325a5b *-
$ git archive --format=tar.gz v1.7.6 | md5sum
a0ca1c873a533a5fcd41d248fb325a5b *-
$ git archive v1.7.6 | gzip -cn >a.tgz && md5sum <a.tgz
a0ca1c873a533a5fcd41d248fb325a5b *-
$ ls -l a.tgz
-rw-r--r-- 1 User Administ 3361584 Aug 1 20:44 a.tgz
$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
30886283af1aed05ae6a36fc5aeda077 *-
$ ls -l a.tgz
-rw-r--r-- 1 User Administ 3374383 Aug 1 20:45 a.tgz
$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
30886283af1aed05ae6a36fc5aeda077 *-
$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
30886283af1aed05ae6a36fc5aeda077 *-
$ git archive v1.7.6 | gzip -cn >a.tgz && md5sum <a.tgz
a0ca1c873a533a5fcd41d248fb325a5b *-
$ git archive --format=tar.gz v1.7.6 | cat >a.tgz && md5sum <a.tgz
a0ca1c873a533a5fcd41d248fb325a5b *-
$ git archive -o a.tgz v1.7.6 && md5sum <a.tgz
30886283af1aed05ae6a36fc5aeda077 *-
So git archive gives the right results when writing to a pipe, but
always the same wrong result when writing directly to a file. With
--format=tar there is no difference:
$ git archive v1.7.6 | md5sum
fa364f38f9128df019dd8fd509c40a6d *-
$ git archive v1.7.6 >a.tar && md5sum <a.tar
fa364f38f9128df019dd8fd509c40a6d *-
René
^ permalink raw reply
* Re: git-svn questions
From: John M. Dlugosz @ 2011-02-24 5:09 UTC (permalink / raw)
To: git
In-Reply-To: <20110801181242.GC10636@sigill.intra.peff.net>
On 8/1/2011 1:12 PM, Jeff King peff-at-peff.net |git/Allow to home| wrote:
> I believe SVN will cache your username (and optionally password, in
> cleartext!) forever in ~/.subversion/auth. I have no idea if Tortoise
> writes to the same area or not. Are you sure you never ran "svn" from
> the command-line, in addition to running Tortoise?
>
> Just a guess. I know very little about how svn works in this respect. :)
>
> -Peff
I only used tortoise. I used its Repo Browser to establish a working directory on my HD.
^ permalink raw reply
* Re: [PATCH] notice error exit from pager
From: Clemens Buchacher @ 2011-08-01 20:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3goj38n.fsf@alter.siamese.dyndns.org>
On Mon, Aug 01, 2011 at 01:17:28PM -0700, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
>
> > Since there is no reliable way to handle error reporting in the
> > parent, produce the output in the child instead.
>
> Hmm, how does this interact with your earlier "error_routine: use parent's
> stderr if exec fails" patch?
The use case addressed in this patch (i.e., the pager) does not
strictly dependent on the previous patch. But Johannes noted that
in general, error output of the child could be redirected, hiding
the execve failure again. The error_routine patch avoids that by
writing to the parent's stderr stream.
Clemens
^ permalink raw reply
* Re: [PATCH] notice error exit from pager
From: Junio C Hamano @ 2011-08-01 20:17 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: git
In-Reply-To: <20110801175921.GA17092@toss>
Clemens Buchacher <drizzd@aon.at> writes:
> Since there is no reliable way to handle error reporting in the
> parent, produce the output in the child instead.
Hmm, how does this interact with your earlier "error_routine: use parent's
stderr if exec fails" patch?
> diff --git a/run-command.c b/run-command.c
> index 5c91f37..a2796c4 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -125,9 +125,6 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
> if (code == 127) {
> code = -1;
> failed_errno = ENOENT;
> - if (!silent_exec_failure)
> - error("cannot run %s: %s", argv0,
> - strerror(ENOENT));
> }
> } else {
> error("waitpid is confused (%s)", argv0);
> @@ -282,14 +279,14 @@ fail_pipe:
> } else {
> execvp(cmd->argv[0], (char *const*) cmd->argv);
> }
> - /*
> - * Do not check for cmd->silent_exec_failure; the parent
> - * process will check it when it sees this exit code.
> - */
> - if (errno == ENOENT)
> + if (errno == ENOENT) {
> + if (!cmd->silent_exec_failure)
> + error("cannot run %s: %s", cmd->argv[0],
> + strerror(ENOENT));
> exit(127);
> - else
> + } else {
> die_errno("cannot exec '%s'", cmd->argv[0]);
> + }
> }
> if (cmd->pid < 0)
> error("cannot fork() for %s: %s", cmd->argv[0],
^ permalink raw reply
* Re: [PATCH] test ls-files with relative paths
From: Junio C Hamano @ 2011-08-01 20:14 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Michael J Gruber, git, rrt, john
In-Reply-To: <20110801180320.GA17556@toss>
Clemens Buchacher <drizzd@aon.at> writes:
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
> ---
> On Sun, Jul 31, 2011 at 05:01:22PM -0700, Junio C Hamano wrote:
>>
>> Sounds sane; tests?
>
> Certainly. I'm not testing partial commit since it uses the same
> codepath as ls-files --error-unmatch anyways.
>
> t/t3005-ls-files-relative.sh | 74 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 74 insertions(+), 0 deletions(-)
> create mode 100755 t/t3005-ls-files-relative.sh
Thanks. This should be a part of the primary patch, not a standalone patch.
> +test_expect_success 'prepare' '
> + : >never-mind-me &&
> + git add never-mind-me &&
> + mkdir top &&
> + (
> + cd top &&
> + mkdir sub &&
> + x="x xa xbc xdef xghij xklmno" &&
> + y=$(echo "$x" | tr x y) &&
> + touch $x &&
> + touch $y &&
These are not meant to be quoted; good.
> + cd sub &&
> + git add ../x*
> + )
> +'
> +
> +test_expect_success 'ls-files with mixed levels' '
> + (
> + cd top/sub &&
> + f=$(cat <<-EOF
> + ../../never-mind-me
> + ../x
> + EOF
> + ) &&
> + echo "$f" >expect &&
> + git ls-files $f >actual &&
This is a funny way to do this; why not cat the here-document into expect
and feed $(cat expect) to ls-files? I suspect that it would become easier
to read and certainly less confusing without need on the readers' side to
scratch their heads wondering "does this $f need to be quoted?" and such.
> +test_expect_success 'ls-files -c' '
> + (
> + cd top/sub &&
> + for f in ../y*
> + do
> + echo "error: pathspec ${sq}${f}${sq} did not match any file(s) known to git."
Why not $sq$f$sq? The above looks harder to read with the braces.
> + set ../x* &&
> + IFS="$new_line" && echo "$*" >>expect && unset IFS &&
That's a funny way to say "ls ../x*" isn't it?
^ permalink raw reply
* Re: Storing additional information in commit headers
From: Jeff King @ 2011-08-01 20:13 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110801182015.GA3100@fishbowl.rw.madduck.net>
On Mon, Aug 01, 2011 at 08:20:15PM +0200, martin f krafft wrote:
> Instead, I am investigating ways in which I can store additional
> information for a branch, and ideally in a way to make it
> transparent and automatic for all users of a project's repo.
>
> Hence, if I were to store additional information in the commit
> object headers, this information would by design be correct,
> immutable, and non-redundant. I am going to reply to my own mail
> with some implementation details to feed the curious, with the hope
> to keep this debate focused.
>
> Are there any strong reasons against my use of commit headers for
> specific, well-defined purposes in contained use-cases? E.g. are
> there tools known to only copy "known" headers, which could
> potentially break my assumptions?
This topic has come up several times in the past few years. I think some
of the relevant questions to consider about your new data are:
1. Does git actually care about your data? E.g., would it want to use
it for reachability analysis in git-fsck?
2. Is it an immutable property of a commit, or can it be changed after
the fact?
If (2) is no, then git-notes is probably the best choice.
Otherwise, if (1) is yes, then a commit header makes sense. But then, it
should also be something that git is taught about, and your commit
header should not be some topgit-specific thing, but a header showing
the generalized form.
Otherwise, the usual recommendation is to use a pseudo-header within the
body of the commit message (i.e., "Topgit-Base: ..." at the end of the
commit message). The upside is that it's easy to create, manipulate, and
examine using existing git tools. The downside is that it is something
that the user is more likely to see in "git log" or when editing a
rebased commit message.
Just about every discussion on this topic ends with the pseudo-header
recommendation. The only exceptions AFAIK are "encoding" (which git
itself needs to care about), and "generation" (which, as you noted,
raises other questions).
-Peff
^ permalink raw reply
* Re: Storing additional information in commit headers
From: Clemens Buchacher @ 2011-08-01 20:01 UTC (permalink / raw)
To: martin f krafft; +Cc: Sverre Rabbelier, git discussion list, Petr Baudis
In-Reply-To: <20110801183411.GA5182@fishbowl.rw.madduck.net>
On Mon, Aug 01, 2011 at 08:34:11PM +0200, martin f krafft wrote:
>
> Yes, I have considered git-notes. The issue I have with git-notes is
> that it requires every contributor to set up refspecs for fetch and
> push, or else the notes will not be exchanged/shared.
Notes are tracked using a 'branch' too. It's just a branch in the
refs/notes namespace, the notes ref. You could simply tag your
notes ref or point a ref from the refs/heads namespace to it each
time you create new notes.
> Also, git-notes are mutable (at least from the UI perspectiv) and
> I strive to encode information immutably.
Notes are also used by textconv, for example, to cache immutable
data. It's not likely a user will end up editing it by accident
unless you use the default notes ref.
Clemens
^ permalink raw reply
* Re: tracking submodules out of main directory.
From: Jens Lehmann @ 2011-08-01 19:39 UTC (permalink / raw)
To: henri GEIST; +Cc: Alexei Sholik, Junio C Hamano, git, Sverre Rabbelier
In-Reply-To: <1312062927.3261.334.camel@Naugrim.eriador.com>
Am 30.07.2011 23:55, schrieb henri GEIST:
> Le samedi 30 juillet 2011 à 16:16 +0200, Jens Lehmann a écrit :
>> Am 29.07.2011 11:39, schrieb henri GEIST:
>>> Le jeudi 28 juillet 2011 à 18:48 +0200, Jens Lehmann a écrit :
>>>> Am 28.07.2011 10:57, schrieb henri GEIST:
>>>
>>> It is not a matter of disabling any control of git in its own
>>> repository.
>>> It is just a matter of adding inside the git repository a reference
>>> (dependency) to an other git repository.
>>
>> ... which you want to have *outside* of the containing repository!
>
> yes
>
>> That will then be registered in other git repositories too in your model,
>> which gets rid of the "one file/submodule, one repo" assumption we now have
>> and will introduce ambiguities which are *really* hard to handle.
>
> I am sorry, I am not a native English speaker. This sentence is to
> complex for me. And google translator is of no help in this case.
Your proposal of letting multiple gitlinks in different repos point to the
same submodule will break the assumption that each file is only handled by
a single git repo. For example when you have a conflict and do a "git
submodule update --recursive" in the superproject, the SHA1 used for "lib"
will depend on the alphabetical order of "project1" and "project2". And
normally after running "git submodule update --recursive" you expect all
submodules of the superproject to be clean. But your change breaks this
expectation, it will still contain unclean submodule entries even though
you just told git it should clean them. What will a "git submodule sync
--recursive" do when "project1" and "project2" use different urls in their
.gitmodules? And so on.
Commands won't always behave like you expect them to and sometimes will
give different results just because different names are used. That's what
I meant with ambiguities and that's why I don't think gitlinks are the
right method here.
> But I agree the step is really weak before enabling to put any regular
> file outside of the directory.
> I do not see any reasonable workflow (to my eyes) for it but' maybe some
> day someone will came with a justifiable workflow which need it. we will
> never know.
>
> But in this case we need solve some questions :
> - Will we extend git status signaling untracked files out of the
> repository ?
I don't think that would work well.
> - What will do git-clean ? it is already dangerous inside the
> repository. and it will be worst if it can access outside of it.
Hopefully git clean will learn the --recurse-submodules option in the not
too distant future, then you will have just the same danger for the files
inside a submodule.
>>> Because in this case it is not just a reference that is managed but the
>>> file itself. And this way there is a risk to overwrite some data not
>>> under revision control outside of the repository.
>>
>> You have the same risk when a gitlink points outside, as a submodule is a
>> way of controlling a bunch of files through that reference. And the file
>> would be under version control in the repository where it is registered, no?
>
> I agree on this point.
>
> But they are still confined in an another git repository not
> disseminated all over the file system.
> And it never corrupt this pointed repository. just ask it to do by it's
> own regular git commands.
The only difference here is that a submodule can contain more than one file,
but you can corrupt those files just as easily as a single file using git
commands.
> In fact you can argue that it can disseminate some complete git
> repository anywhere in the file system.
> And you will be right. (nothing is perfect.)
I'm not concerned about not being perfect (nothing is perfect), but it is
dangerous.
> I can do a second patch to prevent git submodule command to make clones
> outside of the repository.
> It will requires the user to do those clones manually.
> In fact this is already what I do.
> My only use of this is to track dependencies.
But gitlinks are more than simple dependencies, they are followed! "git
submodule", status, diff and fetch already follow them. push is learning
that right now. checkout, reset, merge and friends are being taught that
too (see the enhance_submodule branch in my github repo for the current
state). So a gitlink is more than just a simple reference, it is followed
by a lot of commands and the submodule it points to is manipulated by
those commands. We had a patch for "git archive --recurse-submodules" on
the list, what will that do when used in "project1"?
>>> And in fact it is just what I want, it enable me if I decide to work on
>>> an optional "BigProject" depending on both project 1 & 2.
>>>
>>> Then If lib1 is in version M:
>>> - a git status in project2 will say nothing
>>> - a git status in project1 will say
>>> "modified: ../lib1 (modified content)
>>> - a git status in BigProject will say
>>> "modified: ../project1 (modified content)
>>>
>>> Then I know that I need to update project1 to work with the last version
>>> M of lib1.
>>
>> Maybe no update for project1 is needed, because M only contains a bugfix
>> which doesn't even need a recompilation of project1. But now you need to
>> add a commit to project1 nonetheless with a message like "Updated lib1
>> with a bugfix which is needed by project2" which makes your idea of
>> independent submodules break down.
>
> In fact I work ni the world of "high integrity programming" then It is
> just what I need.
> If there is a bugfix in any library, used by the program it is no more
> the same program.
> I need the "SHA1" to correspond to the exact and complete source code
> involved in my executable.
>
> And this way the "SHA1" of the project sign the "SHA1" of the
> libraries.
I cannot believe you want single commits in your "Gimp" repo for every
combination of distributions and library versions where someone said
"this works". This is insane and won't scale at all.
What you do is that each distribution tests their combination of programs
and libraries and says "that works". And that is why the only sane way to
record this "high integrity programming" test result is in the superproject
(= distribution) and not in each of the program repositories.
I also see that it would be cool when a program could record "I do work with
that library version, if you use another you are on your own". But it will
never say "I only work with *this* specific library version", which is what
your proposal is trying to do.
>>>> You are opening a can of worms by having two different repos point to the same
>>>> submodule living in a third repo (which also happens to be their superproject
>>>> and must somehow ignore it). You'll have two SHA1s for a single submodule;
>>>> "git submodule foreach --recursive" will have interesting results too; and so
>>>> on. Not good.
>>>
>>> As I just said before it is my purpose to do it like that.
>>
>> I understood that, but what are you proposing to do to solve all the
>> problems your approach introduces? You can't just hand wave them away.
>
> There is some solutions :
>
> - First it is one more **feature** if it does not correspond to your
> work flow it does not prevent you to work exactly the way you did
> until now.
>
> - Second if you want to use the feature but not want to have the
> conflict **feature** (for me it is one), just put the independent
> project with there libs in different directory
>
> -+- foo -+- lib1 (in version N)
> | +- project1
> |
> +- bar -+- lib1 (in version M)
> +- project2
>
> - Third if you really need to have project 1 & 2 in the same
> directory foo, that means they are needed by a third BigProject in
> the same directory foo depending on project 1 & 2.
> And then you really need git to declare a conflict.
No you don't. You just need to git to tell you: this is not the version I
was tested against, repeat the tests to be sure.
>>> Let say a concret exemple
>>>
>>> 3 different teams work on libtiff, libpng, and libjpeg they are totally
>>> unrelated.
>>>
>>> One more team is working on the "gimp". And they need those 3 libs in
>>> specific versions not necessarily there heads.
>>>
>>> One other unrelated team is working on "gqview" and need the same libs
>>> in other specifics versions (Why should they know what te gimp team
>>> does)
>>>
>>> Neither "gimp" and "gqview" project will contain directory with those
>>> libs inside. They just depend on them.
>>>
>>> And the last team work on the gnome project which need the "gimp" and
>>> "gqview". It will be this team witch have to care about having both
>>> "gimp" and "gqview" sharing the same libs version>
>>> And has well the gnome project will not contain "gqview" and "gimp" in
>>> its own tree.
>>> It will also depend on them.
>>
>> Cool, that is a real life example resembling what we have a my dayjob. But
>> a "gimp" and "gqview" project will only have dependencies like "use libpng
>> of version 1.2.3 or newer (because we need a feature/bugfix introduced
>> there)" and won't be tied to a special version of that library. This means
>> they need a dependency like "SHA1 or newer" instead of "exactly this SHA1".
>
> It is useful and simpler to work like this but could introduce some
> bugs.
But that model is awfully successful and is used by all distributions I know,
so I suspect it is not that dangerous (especially when you do your own QA).
> The "gimp" team has tested it with libpng 1.2.3 and maybe know that it
> did not work with previous versions but if they do not have any crystal
> ball they never know if newer versions will not break something.
> In fact I doubt that the first version of gimp will work with the last
> version of libpng.
But in the real world it is exactly like that: gimp will work with all libpng
1.2.3 and newer, only when libpng is updated to 2.0.0 you have to check that
again. Of course there will be bugs in some combinations. But the advantage of
being able to then only fix libpng and have the bug fixed in Gimp without
having to change it is far greater than the possible problem you are describing
here.
>>> It is just the same with aptitude on debian.
>>> Each package know there dependency by themselves, does not contain there
>>> dependencies, and do not need a bigger superpackage to tell them what
>>> are there own dependencies.
>>
>> And this is a very good point for the "version x.yy-z *or newer*" argument,
>> they are /never/ tied to the /exact/ x.yy-z version, as that would make the
>> dependencies pretty much unusable. They use a "newer than x.yy-z" scheme.
>
> It is an other feature that the one I need.
> But it is a good idea.
>
> Nothing prevent us to make a patch to add a new test in git status to
> see if the current SHA1 in the libpng repository has the SHA1 of the
> gitlink in the gimp in its ancestor.
To make that feature useful for others (e.g. at my dayjob) this would be
necessary. And we would never want the exact SHA1 match, even though that
information might be what others (like you) want.
>>> And Still I realy want to have every project knowing there own
>>> dependency by themselves and not needing an external superproject to
>>> tell them what they need.
>>
>> I want to have that too! I'm just convinced using a gitlink to achieve that
>> is wrong in so many ways. I'd rather prefer to express such dependencies in
>> something like a config file, and I believe they should not be as strict as
>> "I need exactly that version" but rather like "this version or newer (and by
>> the way: we of course only tested that specific version ;-)". These
>> dependencies could then be checked and displayed by git status.
>
> It effectively could be in a config file it seem good to me as well.
Ok.
> But if git handle this config file.
> Update it on a "git add ../libpng && git commit"
I'm not sure an automatic update at "git commit" would be the right thing to
do, as I think that should only happen after all tests have run successful,
not at the time you commit it. But anyways, that could be done with a post
commit hook. Or the test script can do it when it succeeded.
> And control the matching between the project and libraries on
> "git status">
An extension to "git status" to display the dependencies that aren't met is
a valid goal. What about starting with a script ("git depends"?) and then see
what can go into status?
> I can not see the difference with a gitlink.
Then you can just use a config file for that, no? ;-)
^ permalink raw reply
* Re: Storing additional information in commit headers
From: Martin Langhoff @ 2011-08-01 19:33 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110801182845.GA4303@fishbowl.rw.madduck.net>
On Mon, Aug 1, 2011 at 2:28 PM, martin f krafft <madduck@madduck.net> wrote:
> For lack of a better idea (cf. [0]), I am currently toying with the
> following approach:
Hi Martin!
What data are you trying to include? Some time ago, I had similar
ideas to yours for a while... and it ended up being that all I needed
was to put the additional data /in a file/ and commit that file.
Speculation: you mention distro packaging, so I assume you're
improving the Debian packaging integration, with git tracking
debian/rules, perhaps with a wrapper. If you are using a wrapper
program, it is trivial to update this "metadata" file, or to ensure
it's valid/sane, in the preparations to commit, perhaps ensuring that
a pre-commit-hook script is in place and executable.
hth,
m
--
martin.langhoff@gmail.com
martin@laptop.org -- Software Architect - OLPC
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
^ permalink raw reply
* Re: [PATCH] For Real - Fixed pluralization in diff reports
From: Jeff King @ 2011-08-01 18:38 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jon Forrest, git, gitster
In-Reply-To: <CAGdFq_j0Hr4LMaFC3irS8rhv2N0ePz_t8xpNTDCA-arC-E2UDA@mail.gmail.com>
On Mon, Aug 01, 2011 at 08:32:51PM +0200, Sverre Rabbelier wrote:
> On Mon, Aug 1, 2011 at 20:27, Jon Forrest <nobozo@gmail.com> wrote:
> > I entirely agree. My point is only that the various tests
> > that expect the current behavior will have to be changed
> > whether the implementation of correct plurals uses my
> > inferior method or the way more correct i18n method.
>
> Wouldn't it be a nice hack if we just solved problem through i18n
> then? Have all the plumbing see the current wording, but through i18n
> change it to something grammatically correct for the porcelain.
> Probably not possible, but a nice daydream :).
I thought there was still some question of whether this text was
something that should be script-parseable. If it is, then it shouldn't
be i18n'd at all, nor should we lightly change the format with
pluralization magic. And if it isn't, then we should definitely go the
full i18n route. So in either case, the original patch isn't
appropriate.
I don't have a strong opinion myself. I tend to lean towards i18n-ing
it, because any scripts should be using --numstat to parse, anyway.
OTOH, as Junio pointed out, we are matching the output of much older
tools, so pre-git scripts might be written to read the --stat format.
I've never seen such a script, and I have no idea how many there really
are.
-Peff
^ permalink raw reply
* Re: Storing additional information in commit headers
From: martin f krafft @ 2011-08-01 18:34 UTC (permalink / raw)
To: Sverre Rabbelier, git discussion list, Petr Baudis,
Clemens Buchacher
In-Reply-To: <CAGdFq_h5gimMkwhyOv0PFqVwx4pExL7Zri8Tki_dT5HDtFcBFA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]
also sprach Sverre Rabbelier <srabbelier@gmail.com> [2011.08.01.2027 +0200]:
> On Mon, Aug 1, 2011 at 20:20, martin f krafft <madduck@madduck.net> wrote:
> > My intent was to add an extra commit header to select commits as
> > a way to store extra information needed to automate the management
> > of interdependent branches and patch generation à la TopGit.
>
> Have you had a look at git notes?
Hello, and thanks for taking the time to reply to me!
Yes, I have considered git-notes. The issue I have with git-notes is
that it requires every contributor to set up refspecs for fetch and
push, or else the notes will not be exchanged/shared.
I realise this is a minor concern to most of you, or maybe even
a feature (part of the beauty of Git is, after all, that it works
without requiring everyone to have the same local setup), but in our
use-case (distro packaging), it's a relatively large burden to new
contributors and passerby's (sp?).
Also, git-notes are mutable (at least from the UI perspectiv) and
I strive to encode information immutably.
Therefore I am looking for a means to encode this (necessary)
information as part of the main DAG (i.e. not polluting the
worktree).
I hope this makes sense.
--
martin | http://madduck.net/ | http://two.sentenc.es/
"first get your facts; then you can distort them at your leisure."
-- mark twain
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/sig-policy/999bbcc4/current) --]
[-- Type: application/pgp-signature, Size: 1124 bytes --]
^ permalink raw reply
* Re: [PATCH] For Real - Fixed pluralization in diff reports
From: Sverre Rabbelier @ 2011-08-01 18:32 UTC (permalink / raw)
To: Jon Forrest; +Cc: Jeff King, git, gitster
In-Reply-To: <4E36F025.9040100@gmail.com>
Heya,
On Mon, Aug 1, 2011 at 20:27, Jon Forrest <nobozo@gmail.com> wrote:
> I entirely agree. My point is only that the various tests
> that expect the current behavior will have to be changed
> whether the implementation of correct plurals uses my
> inferior method or the way more correct i18n method.
Wouldn't it be a nice hack if we just solved problem through i18n
then? Have all the plumbing see the current wording, but through i18n
change it to something grammatically correct for the porcelain.
Probably not possible, but a nice daydream :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 17/18] revert: Introduce --continue to continue the operation
From: Ramkumar Ramachandra @ 2011-08-01 18:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312222025-28453-18-git-send-email-artagnon@gmail.com>
Hi again,
Ramkumar Ramachandra writes:
> +test_expect_success 'malformed instruction sheet 1' '
> + pristine_detach initial &&
> + test_must_fail git cherry-pick base..anotherpick &&
> + echo "resolved" >foo &&
> + git add foo &&
> + git commit &&
> + sed "s/pick /pick/" .git/sequencer/todo >new_sheet
> + cp new_sheet .git/sequencer/todo
> + test_must_fail git cherry-pick --continue
> +'
Ugh. I forgot to chain using '&&' .
> +test_expect_success 'malformed instruction sheet 2' '
> + pristine_detach initial &&
> + test_must_fail git cherry-pick base..anotherpick &&
> + echo "resolved" >foo &&
> + git add foo &&
> + git commit &&
> + sed "s/pick/revert/" .git/sequencer/todo >new_sheet
> + cp new_sheet .git/sequencer/todo
> + test_must_fail git cherry-pick --continue
> +'
Here too.
Please squash this in while reading -- thanks :)
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 457ad76..626136a 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -195,8 +195,8 @@ test_expect_success 'malformed instruction sheet 1' '
echo "resolved" >foo &&
git add foo &&
git commit &&
- sed "s/pick /pick/" .git/sequencer/todo >new_sheet
- cp new_sheet .git/sequencer/todo
+ sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
test_must_fail git cherry-pick --continue
'
@@ -206,8 +206,8 @@ test_expect_success 'malformed instruction sheet 2' '
echo "resolved" >foo &&
git add foo &&
git commit &&
- sed "s/pick/revert/" .git/sequencer/todo >new_sheet
- cp new_sheet .git/sequencer/todo
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
test_must_fail git cherry-pick --continue
'
-- Ram
^ permalink raw reply related
* Re: Storing additional information in commit headers
From: martin f krafft @ 2011-08-01 18:28 UTC (permalink / raw)
To: git discussion list; +Cc: Petr Baudis, Clemens Buchacher
In-Reply-To: <20110801182015.GA3100@fishbowl.rw.madduck.net>
[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]
also sprach martin f krafft <madduck@madduck.net> [2011.08.01.2020 +0200]:
> Hence, if I were to store additional information in the commit
> object headers, this information would by design be correct,
> immutable, and non-redundant. I am going to reply to my own mail
> with some implementation details to feed the curious, with the hope
> to keep this debate focused.
For lack of a better idea (cf. [0]), I am currently toying with the
following approach:
Possibly in addition to the orphan parent pointer to a commit object
suggested in [0], and in order to provide a clear means to identify
said orphan parent pointer (holding additional information), I am
considering storing this orphan parent commit's ref in the main
commit, using a header like x-topgit-top-base [1].
0. http://permalink.gmane.org/gmane.comp.version-control.git/178349
1. The use of the x- prefix is obviously intentional to suggest that
this is a free-form, non-standard extension.
Whenever the extra data need changing, a new x-topgit-top-base ref
is added to HEAD.
Now, given a commitish, I simply have to walk back in time until
I find a commit object with such a header, and I have the most
recent metadata at my fingertips.
Instead of a ref to the orphan parent commit (which visibily
pollutes the history), I could also just store the information right
there.
This is arguably hackish, but unless I find a better way, it's the
best I've come up with thus far.
And of course, this could go into the commit message body text,
but it being an implementation detail, that's really not the right
place for it.
Thanks for your consideration,
--
martin | http://madduck.net/ | http://two.sentenc.es/
"there are two major products that come out of berkeley: lsd and unix."
one caused me an addiction
-- fyodor
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/sig-policy/999bbcc4/current) --]
[-- Type: application/pgp-signature, Size: 1124 bytes --]
^ permalink raw reply
* Re: Storing additional information in commit headers
From: Sverre Rabbelier @ 2011-08-01 18:27 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110801182015.GA3100@fishbowl.rw.madduck.net>
Heya,
On Mon, Aug 1, 2011 at 20:20, martin f krafft <madduck@madduck.net> wrote:
> My intent was to add an extra commit header to select commits as
> a way to store extra information needed to automate the management
> of interdependent branches and patch generation à la TopGit.
Have you had a look at git notes?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] For Real - Fixed pluralization in diff reports
From: Jon Forrest @ 2011-08-01 18:27 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
In-Reply-To: <20110801180603.GB10636@sigill.intra.peff.net>
On 8/1/2011 11:06 AM, Jeff King wrote:
>
> I think he means that auto-pluralization like this cannot be done in an
> i18n world, as many languages do not simply add "s". Your patch would
> have to use ngettext, something like this (totally untested and just
> copying a similar spot in suggest_reattach, as I have never done any
> i18n myself):
[snip]
> And that gives translators a chance to specify the entire singular and
> plural versions separately.
I entirely agree. My point is only that the various tests
that expect the current behavior will have to be changed
whether the implementation of correct plurals uses my
inferior method or the way more correct i18n method.
Jon
^ permalink raw reply
* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-08-01 18:25 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: Junio C Hamano, git, j6t
In-Reply-To: <CABPQNSbyp2hEgXvzo3bMHHxrbc9fQLqaABweqfjP7xQzBUdj6A@mail.gmail.com>
On Mon, Aug 01, 2011 at 08:02:22PM +0200, Erik Faye-Lund wrote:
> > Weird. What does j.tgz end up looking like? Is it empty, or does it have
> > bogus data in it? Does gzip actually get invoked at all? Try running
> > with GIT_TRACE=1. I don't suppose you guys have something like strace,
> > which might be helpful.
> >
>
> It does have data, and gzip does get invoked:
> $ gunzip -c j.tgz | wc -c
>
> gzip: j.tgz: invalid compressed data--format violated
> 131072
>
> So it seems there are around 130k of valid data before it barfs.
>
> Hmm, but when I try the same after re-running the test, I get a
> different amount of valid data (491520 bytes this time)... Is this a
> timing-related issue?
Hmm. Non-deterministic output is not good. Could be timing, or we could
be touching memory we're not supposed to. I just ran t5000 through
valgrind, and nothing turned up. And of course I can't replicate the
test failure on Linux. So I assume it really is Windows-specific.
That makes me suspect some difference in how the run-command API works
on the two platforms, since that is the code here that will be vastly
difference. I notice the start_command code uses dup() on Windows
instead of dup2() (I guess you guys don't have dup2?). I wonder if there
could be some issue with another descriptor accidentally pointing to the
same spot. But that's just a wild guess.
I think at this point, I'd probably start stepping through the archive
code with gdb.
-Peff
^ permalink raw reply
* Storing additional information in commit headers
From: martin f krafft @ 2011-08-01 18:20 UTC (permalink / raw)
To: git discussion list; +Cc: Petr Baudis, Clemens Buchacher
[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]
Dear list,
I've read — with great interest — the recent discussion on
generation numbers[0], mostly because Clemens Buchacher pointed me
to it as a warning not to mess with commit objects.
0. http://comments.gmane.org/gmane.comp.version-control.git/177146
My intent was to add an extra commit header to select commits as
a way to store extra information needed to automate the management
of interdependent branches and patch generation à la TopGit.
Having read the generation numbers debate, I am not sure that adding
additional commit headers is a bad idea per se. From what
I understand, the main pushback to Linus' idea was that people did
not feel it right to store redundant, calculateable information
permanently in commit objects, where they cannot be altered anymore,
despite the non-zero chance of there being an error. Instead, the
use of a cache was advocated. I do not want to take a side in this
debate with this mail of mine.
Instead, I am investigating ways in which I can store additional
information for a branch, and ideally in a way to make it
transparent and automatic for all users of a project's repo.
Hence, if I were to store additional information in the commit
object headers, this information would by design be correct,
immutable, and non-redundant. I am going to reply to my own mail
with some implementation details to feed the curious, with the hope
to keep this debate focused.
Are there any strong reasons against my use of commit headers for
specific, well-defined purposes in contained use-cases? E.g. are
there tools known to only copy "known" headers, which could
potentially break my assumptions?
Thanks,
--
martin | http://madduck.net/ | http://two.sentenc.es/
"when a gentoo admin tells me that the KISS principle is good for
'busy sysadmins', and that it's not an evolutionary step backwards,
i wonder whether their tape is already running backwards."
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/sig-policy/999bbcc4/current) --]
[-- Type: application/pgp-signature, Size: 1124 bytes --]
^ permalink raw reply
* Re: [PATCH v11 4/7] bisect: introduce support for --no-checkout option.
From: Junio C Hamano @ 2011-08-01 18:13 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <1312199777-10144-5-git-send-email-jon.seymour@gmail.com>
Jon Seymour <jon.seymour@gmail.com> writes:
> diff --git a/bisect.c b/bisect.c
> index dd7e8ed..534da2c 100644
> --- a/bisect.c
> +++ b/bisect.c
> ...
> - res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
> - if (res)
> - exit(res);
> + if (no_checkout) {
> + argv_update_ref[3] = bisect_rev_hex;
> + if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
> + die("update-ref --no-deref HEAD failed on %s", bisect_rev_hex);
> + } else {
> + res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
> + if (res)
> + exit(res);
> + }
Please. No indentations with just a few SPs.
^ permalink raw reply
* [PATCH 18/18] revert: Propagate errors upwards from do_pick_commit
From: Ramkumar Ramachandra @ 2011-08-01 18:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312222025-28453-1-git-send-email-artagnon@gmail.com>
Currently, revert_or_cherry_pick can fail in two ways. If it
encounters a conflict, it returns a positive number indicating the
intended exit status for the git wrapper to pass on; for all other
errors, it calls die(). The latter behavior is inconsiderate towards
callers, as it denies them the opportunity to recover from errors and
do other things.
After this patch, revert_or_cherry_pick will still return a positive
return value to indicate an exit status for conflicts as before, while
for some other errors, it will print an error message and return -1
instead of die()-ing. The cmd_revert and cmd_cherry_pick are adjusted
to handle the fatal errors by die()-ing themselves.
While the full benefits of this patch will only be seen once all the
"die" calls are replaced with calls to "error", its immediate impact
is to change some "fatal:" messages to say "error:" and to add a new
"fatal: cherry-pick failed" message at the end when the operation
fails.
Inspired-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 86 +++++++++++++++++++++++++-----------------------------
1 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 35ee089..7bb7313 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,15 +66,6 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-static void fatal(const char *advice, ...)
-{
- va_list params;
-
- va_start(params, advice);
- vreportf("fatal: ", advice, params);
- va_end(params);
-}
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -363,25 +354,20 @@ static struct tree *empty_tree(void)
return tree;
}
-static NORETURN void die_dirty_index(struct replay_opts *opts)
+static int error_dirty_index(struct replay_opts *opts)
{
- if (read_cache_unmerged()) {
- die_resolve_conflict(action_name(opts));
- } else {
- if (advice_commit_before_merge) {
- if (opts->action == REVERT)
- die(_("Your local changes would be overwritten by revert.\n"
- "Please, commit your changes or stash them to proceed."));
- else
- die(_("Your local changes would be overwritten by cherry-pick.\n"
- "Please, commit your changes or stash them to proceed."));
- } else {
- if (opts->action == REVERT)
- die(_("Your local changes would be overwritten by revert.\n"));
- else
- die(_("Your local changes would be overwritten by cherry-pick.\n"));
- }
- }
+ if (read_cache_unmerged())
+ return error_resolve_conflict(action_name(opts));
+
+ /* Different translation strings for cherry-pick and revert */
+ if (opts->action == CHERRY_PICK)
+ error(_("Your local changes would be overwritten by cherry-pick."));
+ else
+ error(_("Your local changes would be overwritten by revert."));
+
+ if (advice_commit_before_merge)
+ advise(_("Commit your changes or stash them to proceed."));
+ return -1;
}
static int fast_forward_to(const unsigned char *to, const unsigned char *from)
@@ -499,9 +485,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
die (_("Your index file is unmerged."));
} else {
if (get_sha1("HEAD", head))
- die (_("You do not have a valid HEAD"));
+ return error(_("You do not have a valid HEAD"));
if (index_differs_from("HEAD", 0))
- die_dirty_index(opts);
+ return error_dirty_index(opts);
}
discard_cache();
@@ -514,20 +500,20 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
struct commit_list *p;
if (!opts->mainline)
- die(_("Commit %s is a merge but no -m option was given."),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Commit %s is a merge but no -m option was given."),
+ sha1_to_hex(commit->object.sha1));
for (cnt = 1, p = commit->parents;
cnt != opts->mainline && p;
cnt++)
p = p->next;
if (cnt != opts->mainline || !p)
- die(_("Commit %s does not have parent %d"),
- sha1_to_hex(commit->object.sha1), opts->mainline);
+ return error(_("Commit %s does not have parent %d"),
+ sha1_to_hex(commit->object.sha1), opts->mainline);
parent = p->item;
} else if (0 < opts->mainline)
- die(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Mainline was specified but commit %s is not a merge."),
+ sha1_to_hex(commit->object.sha1));
else
parent = commit->parents->item;
@@ -537,12 +523,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
if (parent && parse_commit(parent) < 0)
/* TRANSLATORS: The first %s will be "revert" or
"cherry-pick", the second %s a SHA1 */
- die(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
+ return error(_("%s: cannot parse parent commit %s"),
+ action_name(opts), sha1_to_hex(parent->object.sha1));
if (get_message(commit, &msg) != 0)
- die(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
/*
* "commit" is an existing commit. We would want to apply
@@ -995,27 +981,28 @@ static int pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
- fatal(_("A cherry-pick or revert is in progress."));
+ error(_("A cherry-pick or revert is in progress."));
advise(_("Use --continue to continue the operation"));
advise(_("or --reset to forget about it"));
- exit(128);
+ return -1;
}
if (get_sha1("HEAD", sha1)) {
if (opts->action == REVERT)
- die(_("Can't revert as initial commit"));
- die(_("Can't cherry-pick into empty head"));
+ return error(_("Can't revert as initial commit"));
+ return error(_("Can't cherry-pick into empty head"));
}
save_head(sha1_to_hex(sha1));
save_opts(opts);
}
return pick_commits(todo_list, opts);
error:
- die(_("No %s in progress"), action_name(opts));
+ return error(_("No %s in progress"), action_name(opts));
}
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
+ int res;
memset(&opts, 0, sizeof(opts));
if (isatty(0))
@@ -1023,16 +1010,23 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_revisions(&opts);
+ res = pick_revisions(&opts);
+ if (res < 0)
+ die(_("revert failed"));
+ return res;
}
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
+ int res;
memset(&opts, 0, sizeof(opts));
opts.action = CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_revisions(&opts);
+ res = pick_revisions(&opts);
+ if (res < 0)
+ die(_("cherry-pick failed"));
+ return res;
}
--
1.7.4.rc1.7.g2cf08.dirty
^ permalink raw reply related
* [PATCH 17/18] revert: Introduce --continue to continue the operation
From: Ramkumar Ramachandra @ 2011-08-01 18:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312222025-28453-1-git-send-email-artagnon@gmail.com>
Introduce a new "git cherry-pick --continue" command which uses the
information in ".git/sequencer" to continue a cherry-pick that stopped
because of a conflict or other error. It works by dropping the first
instruction from .git/sequencer/todo and performing the remaining
cherry-picks listed there, with options (think "-s" and "-X") from the
initial command listed in ".git/sequencer/opts".
So now you can do:
$ git cherry-pick -Xpatience foo..bar
... description conflict in commit moo ...
$ git cherry-pick --continue
error: 'cherry-pick' is not possible because you have unmerged files.
fatal: failed to resume cherry-pick
$ echo resolved >conflictingfile
$ git add conflictingfile && git commit
$ git cherry-pick --continue; # resumes with the commit after "moo"
During the "git commit" stage, CHERRY_PICK_HEAD will aid by providing
the commit message from the conflicting "moo" commit. Note that the
cherry-pick mechanism has no control at this stage, so the user is
free to violate anything that was specified during the first
cherry-pick invocation. For example, if "-x" was specified during the
first cherry-pick invocation, the user is free to edit out the message
during commit time. Note that the "--signoff" option specified at
cherry-pick invocation time is not reflected in the commit message
provided by CHERRY_PICK_HEAD; the user must take care to add
"--signoff" during the "git commit" invocation.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-cherry-pick.txt | 1 +
Documentation/git-revert.txt | 1 +
Documentation/sequencer.txt | 5 +
builtin/revert.c | 184 ++++++++++++++++++++++++++++++++++++-
t/t3510-cherry-pick-sequence.sh | 96 +++++++++++++++++++
5 files changed, 283 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 12a85ba..b4a2d74 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
'git cherry-pick' --reset
+'git cherry-pick' --continue
DESCRIPTION
-----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 31c85b4..68f8e68 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
'git revert' --reset
+'git revert' --continue
DESCRIPTION
-----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 16ce88c..3e6df33 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -2,3 +2,8 @@
Forget about the current operation in progress. Can be used
to clear the sequencer state after a failed cherry-pick or
revert.
+
+--continue::
+ Continue the operation in progress using the information in
+ '.git/sequencer'. Can be used to continue after resolving
+ conflicts in a failed cherry-pick or revert.
diff --git a/builtin/revert.c b/builtin/revert.c
index 09d4b6b..35ee089 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {
};
enum replay_action { REVERT, CHERRY_PICK };
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
struct replay_opts {
enum replay_action action;
@@ -119,14 +119,42 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
va_end(ap);
}
+static void verify_opt_mutually_compatible(const char *me, ...)
+{
+ const char *opt1, *opt2;
+ va_list ap;
+ int set;
+
+ va_start(ap, me);
+ while ((opt1 = va_arg(ap, const char *))) {
+ set = va_arg(ap, int);
+ if (set)
+ break;
+ }
+ if (!opt1)
+ goto ok;
+ while ((opt2 = va_arg(ap, const char *))) {
+ set = va_arg(ap, int);
+ if (set) {
+ va_end(ap);
+ die(_("%s: %s cannot be used with %s"),
+ me, opt1, opt2);
+ }
+ }
+ok:
+ va_end(ap);
+}
+
static void parse_args(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
int noop;
int reset = 0;
+ int contin = 0;
struct option options[] = {
OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
+ OPT_BOOLEAN(0, "continue", &contin, "continue the current operation"),
OPT_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
@@ -156,15 +184,29 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN);
+ /* Check for incompatible subcommands */
+ verify_opt_mutually_compatible(me,
+ "--reset", reset,
+ "--continue", contin,
+ NULL);
+
/* Set the subcommand */
if (reset)
opts->subcommand = REPLAY_RESET;
+ else if (contin)
+ opts->subcommand = REPLAY_CONTINUE;
else
opts->subcommand = REPLAY_NONE;
/* Check for incompatible command line arguments */
- if (opts->subcommand == REPLAY_RESET) {
- verify_opt_compatible(me, "--reset",
+ if (opts->subcommand != REPLAY_NONE) {
+ char *this_operation;
+ if (opts->subcommand == REPLAY_RESET)
+ this_operation = "--reset";
+ else
+ this_operation = "--continue";
+
+ verify_opt_compatible(me, this_operation,
"--no-commit", opts->no_commit,
"--signoff", opts->signoff,
"--mainline", opts->mainline,
@@ -670,6 +712,128 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return 0;
}
+static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+{
+ unsigned char commit_sha1[20];
+ char sha1_abbrev[40];
+ enum replay_action action;
+ int insn_len = 0;
+ char *p, *q;
+
+ if (!prefixcmp(start, "pick ")) {
+ action = CHERRY_PICK;
+ insn_len = strlen("pick");
+ p = start + insn_len + 1;
+ } else if (!prefixcmp(start, "revert ")) {
+ action = REVERT;
+ insn_len = strlen("revert");
+ p = start + insn_len + 1;
+ } else
+ return NULL;
+
+ q = strchr(p, ' ');
+ if (!q)
+ return NULL;
+ q++;
+
+ strlcpy(sha1_abbrev, p, q - p);
+
+ /*
+ * Verify that the action matches up with the one in
+ * opts; we don't support arbitrary instructions
+ */
+ if (action != opts->action) {
+ const char *action_str;
+ action_str = action == REVERT ? "revert" : "cherry-pick";
+ error(_("Cannot %s during a %s"), action_str, action_name(opts));
+ return NULL;
+ }
+
+ if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ return NULL;
+
+ return lookup_commit_reference(commit_sha1);
+}
+
+static void read_populate_todo(struct commit_list **todo_list,
+ struct replay_opts *opts)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ struct strbuf buf = STRBUF_INIT;
+ struct commit_list **next;
+ struct commit *commit;
+ char *p;
+ int fd;
+
+ fd = open(todo_file, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), todo_file);
+ if (strbuf_read(&buf, fd, 0) < 0) {
+ close(fd);
+ strbuf_release(&buf);
+ die(_("Could not read %s."), todo_file);
+ }
+ close(fd);
+
+ next = todo_list;
+ for (p = buf.buf; *p; p = strchr(p, '\n') + 1) {
+ commit = parse_insn_line(p, opts);
+ if (!commit)
+ goto error;
+ next = commit_list_append(commit, next);
+ }
+ if (!*todo_list)
+ goto error;
+ strbuf_release(&buf);
+ return;
+error:
+ strbuf_release(&buf);
+ die(_("Unusable instruction sheet: %s"), todo_file);
+}
+
+static int populate_opts_cb(const char *key, const char *value, void *data)
+{
+ struct replay_opts *opts = data;
+ int error_flag = 1;
+
+ if (!value)
+ error_flag = 0;
+ else if (!strcmp(key, "options.no-commit"))
+ opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.edit"))
+ opts->edit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.signoff"))
+ opts->signoff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.record-origin"))
+ opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.allow-ff"))
+ opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.mainline"))
+ opts->mainline = git_config_int(key, value);
+ else if (!strcmp(key, "options.strategy"))
+ git_config_string(&opts->strategy, key, value);
+ else if (!strcmp(key, "options.strategy-option")) {
+ ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+ opts->xopts[opts->xopts_nr++] = xstrdup(value);
+ } else
+ return error(_("Invalid key: %s"), key);
+
+ if (!error_flag)
+ return error(_("Invalid value for %s: %s"), key, value);
+
+ return 0;
+}
+
+static void read_populate_opts(struct replay_opts **opts_ptr)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (!file_exists(opts_file))
+ return;
+ if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
+ die(_("Malformed options sheet: %s"), opts_file);
+}
+
static void walk_revs_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
{
@@ -813,6 +977,15 @@ static int pick_revisions(struct replay_opts *opts)
if (opts->subcommand == REPLAY_RESET) {
remove_sequencer_state(1);
return 0;
+ } else if (opts->subcommand == REPLAY_CONTINUE) {
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ goto error;
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list, opts);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
} else {
/*
* Start a new cherry-pick/ revert sequence; but
@@ -823,7 +996,8 @@ static int pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
fatal(_("A cherry-pick or revert is in progress."));
- advise(_("Use --reset to forget about it"));
+ advise(_("Use --continue to continue the operation"));
+ advise(_("or --reset to forget about it"));
exit(128);
}
if (get_sha1("HEAD", sha1)) {
@@ -835,6 +1009,8 @@ static int pick_revisions(struct replay_opts *opts)
save_opts(opts);
}
return pick_commits(todo_list, opts);
+error:
+ die(_("No %s in progress"), action_name(opts));
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index fe65d89..457ad76 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -115,4 +115,100 @@ test_expect_success 'cherry-pick does not implicitly stomp an existing operation
test_cmp expect actual
'
+test_expect_success '--continue complains when no cherry-pick is in progress' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success '--continue complains when there are unresolved conflicts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..picked &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success '--continue continues after conflicts are resolved' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue respects opts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "cherry picked from" initial_msg &&
+ grep "cherry picked from" unrelatedpick_msg &&
+ grep "cherry picked from" picked_msg &&
+ grep "cherry picked from" anotherpick_msg
+'
+
+test_expect_success '--signoff is not automatically propogated to resolved conflict' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick --signoff base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "Signed-off-by:" initial_msg &&
+ grep "Signed-off-by:" unrelatedpick_msg &&
+ test_must_fail grep "Signed-off-by:" picked_msg &&
+ grep "Signed-off-by:" anotherpick_msg
+'
+
+test_expect_success 'malformed instruction sheet 1' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick /pick/" .git/sequencer/todo >new_sheet
+ cp new_sheet .git/sequencer/todo
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success 'malformed instruction sheet 2' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet
+ cp new_sheet .git/sequencer/todo
+ test_must_fail git cherry-pick --continue
+'
+
test_done
--
1.7.4.rc1.7.g2cf08.dirty
^ permalink raw reply related
* [PATCH 16/18] revert: Don't implicitly stomp pending sequencer operation
From: Ramkumar Ramachandra @ 2011-08-01 18:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312222025-28453-1-git-send-email-artagnon@gmail.com>
Protect the user from forgetting about a pending sequencer operation
by immediately erroring out when an existing cherry-pick or revert
operation is in progress like:
$ git cherry-pick foo
... conflict ...
$ git cherry-pick moo
error: .git/sequencer already exists
hint: A cherry-pick or revert is in progress
hint: Use --reset to forget about it
fatal: cherry-pick failed
A naive version of this would break the following established ways of
working:
$ git cherry-pick foo
... conflict ...
$ git reset --hard # I actually meant "moo" when I said "foo"
$ git cherry-pick moo
$ git cherry-pick foo
... conflict ...
$ git commit # commit the resolution
$ git cherry-pick moo # New operation
However, the previous patches "reset: Make reset remove the sequencer
state" and "revert: Remove sequencer state when no commits are
pending" make sure that this does not happen.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 30 +++++++++++++++++++++++++-----
t/t3510-cherry-pick-sequence.sh | 9 +++++++++
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index ce57301..09d4b6b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,6 +66,15 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+static void fatal(const char *advice, ...)
+{
+ va_list params;
+
+ va_start(params, advice);
+ vreportf("fatal: ", advice, params);
+ va_end(params);
+}
+
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -675,13 +684,15 @@ static void walk_revs_populate_todo(struct commit_list **todo_list,
next = commit_list_append(commit, next);
}
-static void create_seq_dir(void)
+static int create_seq_dir(void)
{
const char *seq_dir = git_path(SEQ_DIR);
- if (!(file_exists(seq_dir) && is_directory(seq_dir))
- && mkdir(seq_dir, 0777) < 0)
+ if (file_exists(seq_dir))
+ return error(_("%s already exists."), seq_dir);
+ else if (mkdir(seq_dir, 0777) < 0)
die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+ return 0;
}
static void save_head(const char *head)
@@ -803,9 +814,18 @@ static int pick_revisions(struct replay_opts *opts)
remove_sequencer_state(1);
return 0;
} else {
- /* Start a new cherry-pick/ revert sequence */
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+
walk_revs_populate_todo(&todo_list, opts);
- create_seq_dir();
+ if (create_seq_dir() < 0) {
+ fatal(_("A cherry-pick or revert is in progress."));
+ advise(_("Use --reset to forget about it"));
+ exit(128);
+ }
if (get_sha1("HEAD", sha1)) {
if (opts->action == REVERT)
die(_("Can't revert as initial commit"));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 0b68397..fe65d89 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -106,4 +106,13 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
test_cmp expect actual
'
+test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test-chmtime -v +0 .git/sequencer >expect &&
+ test_must_fail git cherry-pick unrelatedpick &&
+ test-chmtime -v +0 .git/sequencer >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.4.rc1.7.g2cf08.dirty
^ permalink raw reply related
* [PATCH 15/18] revert: Remove sequencer state when no commits are pending
From: Ramkumar Ramachandra @ 2011-08-01 18:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312222025-28453-1-git-send-email-artagnon@gmail.com>
When cherry-pick or revert is called on a list of commits, and a
conflict encountered somewhere in the middle, the data in
".git/sequencer" is required to continue the operation. However, when
a conflict is encountered in the very last commit, the user will have
to "continue" after resolving the conflict and committing just so that
the sequencer state is removed. This is how the current "rebase -i"
script works as well.
$ git cherry-pick foo..bar
... conflict encountered while picking "bar" ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git cherry-pick --continue # This would be a no-op
Change this so that the sequencer state is cleared when a conflict is
encountered in the last commit. Incidentally, this patch makes sure
that some existing tests don't break when features like "--reset" and
"--continue" are implemented later in the series.
A better way to implement this feature is to get the last "git commit"
to remove the sequencer state. However, that requires tighter
coupling between "git commit" and the sequencer, a goal that can be
pursued once the sequencer is made more general.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 12 +++++++++++-
t/t3510-cherry-pick-sequence.sh | 24 ++++++++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 2ddd7e7..ce57301 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -765,8 +765,18 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
res = do_pick_commit(cur->item, opts);
- if (res)
+ if (res) {
+ if (!cur->next)
+ /*
+ * An error was encountered while
+ * picking the last commit; the
+ * sequencer state is useless now --
+ * the user simply needs to resolve
+ * the conflict and commit
+ */
+ remove_sequencer_state(0);
return res;
+ }
}
/*
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index a3427a5..0b68397 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -82,4 +82,28 @@ test_expect_success '--reset cleans up sequencer state' '
test_path_is_missing .git/sequencer
'
+test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..picked &&
+ test_path_is_missing .git/sequencer &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.7.4.rc1.7.g2cf08.dirty
^ permalink raw reply related
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