* Re: [PATCH 4/5] sequencer: handle single commit pick separately
From: Jonathan Nieder @ 2011-11-06 0:35 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <1320510586-3940-5-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Don't write a '.git/sequencer/todo', as CHERRY_PICK_HEAD already
> contains this information. However, '.git/sequencer/opts' and
> '.git/sequencer/head' are required to support '--reset' and
> '--continue' operations.
This is meant as a signal to later "git cherry-pick" commands that it
is okay to forget about the cherry-pick, right? How is the reader
supposed to know that? Say so!
By the way, it's not clear to me yet whether the resulting UI would be
more pleasant or not. What is the expected calling sequence? Any odd
corners of behavior changing? What happens if I do
git cherry-pick foo; # conflicts!
git cherry-pick bar; # just ignore them
or
git cherry-pick foo; # conflicts! but resolved in index by rerere
git checkout something-else
Is there any potential downside to the change?
[...]
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -746,6 +746,15 @@ static int pick_commits(struct replay_insn_list *todo_list,
> opts->record_origin || opts->edit));
> read_and_refresh_cache(opts);
>
> + /*
> + * Backward compatibility hack: when only a single commit is
> + * picked, don't save_todo(), because CHERRY_PICK_HEAD will
> + * contain this information anyway.
> + */
How does saving disk space by avoiding saving redundant information
affect backward compatibility? I'm not sure what this comment is
trying to say.
> + if (opts->subcommand == REPLAY_NONE &&
> + todo_list->next == NULL && todo_list->action == REPLAY_PICK)
> + return do_pick_commit(todo_list->operand, REPLAY_PICK, opts);
> +
> for (cur = todo_list; cur; cur = cur->next) {
^ permalink raw reply
* Re: [PATCH] Add abbreviated commit hash to rebase conflict message
From: Junio C Hamano @ 2011-11-06 0:31 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Ævar Arnfjörð Bjarmason, Jonas Flodén,
Junio C Hamano, Eric Herman, Fernando Vezzosi, Git List
In-Reply-To: <1320501759-27236-1-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> Also move the $msgnum to a more sensible location.
>
> Before:
> Patch failed at 0001 msg
> After:
> Patch 0001 failed at [da65151] msg
We can guess that 7-hexdigit is an abbreviated commit object name but the
above description and the title do not tell the most important thing. What
commit are you trying to describe, and why is it a good idea to show it?
> Reviewed-by: Eric Herman <eric@freesa.org>
> Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
I wouldn't have issues if these were Helped-by or Asked-by or something,
but a patch with Reviewed-by for which I do not see any trace of
discussion on this list triggers some WTF at least for me.
Where did these reviews take place? What were their inputs and how was the
patch improved based on them? Why I should trust the judgements of these
people?
What happens when threeway is not enabled, and especially when "git am" is
used for applying patches, not within rebase?
> ---
> git-am.sh | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 9042432..9d70588 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -837,7 +837,8 @@ did you forget to use 'git add'?"
> fi
> if test $apply_status != 0
> then
> - eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
> + abbrev_commit=$(git log -1 --pretty=%h $commit)
> + eval_gettextln 'Patch $msgnum failed at [$abbrev_commit] $FIRSTLINE'
^ permalink raw reply
* Re: [PATCH 3/5] sequencer: sequencer state is useless without todo
From: Jonathan Nieder @ 2011-11-06 0:26 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <1320510586-3940-4-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Later in the series, we will not write '.git/sequencer/todo' for a
> single commit cherry-pick, because 'CHERRY_PICK_HEAD' already contains
> this information. So, stomp the sequencer state in create_seq_state()
> unless the todo file is present.
What problem does this solve? How does it solve it? What does it
mean to stomp?
The usual commit-message debugging strategy applies here: imagine you
are a BIOS clone manufacturer, and for legal reasons you are not
allowed to read this part of the git implementation embedded in the
standard BIOS. However, you are allowed to read the commit message,
and if that message is clear enough, it will explain the purpose and
behavior of that code and you will be able to implement a compatible
implementation addressing the same problem without scratching your
head too much.
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -654,11 +654,15 @@ static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
>
> static int create_seq_dir(void)
> {
> + const char *todo_file = git_path(SEQ_TODO_FILE);
> const char *seq_dir = git_path(SEQ_DIR);
Scary idiom.
> - if (file_exists(seq_dir))
> - return error(_("%s already exists."), seq_dir);
> - else if (mkdir(seq_dir, 0777) < 0)
> + if (file_exists(todo_file))
> + return error(_("%s already exists."), todo_file);
> +
> + /* If todo_file doesn't exist, discard sequencer state */
> + remove_sequencer_state(1);
> + if (mkdir(seq_dir, 0777) < 0)
> die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
I guess this patch would make more sense after patch 4.
^ permalink raw reply
* Re: [PATCH 2/1] gc --auto: warn gc will soon run, give users a chance to run manually
From: Junio C Hamano @ 2011-11-06 0:19 UTC (permalink / raw)
To: Fernando Vezzosi; +Cc: git
In-Reply-To: <20111105154411.079F69004A@inscatolati.net>
Fernando Vezzosi <buccia@repnz.net> writes:
> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
> ---
>
> Rebased Nguyễn's patch on top of mine.
You don't have to do this.
^ permalink raw reply
* Re: [PATCH 2/5] sequencer: remove CHERRY_PICK_HEAD with sequencer state
From: Jonathan Nieder @ 2011-11-06 0:15 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <1320510586-3940-3-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Make remove_sequencer_state() remove '.git/CHERRY_PICK_HEAD' when
> invoked aggressively, since we want to treat it as part of the
> sequencer state now. While at it, make some minor improvements to the
> function.
What does it mean to invoke a function aggressively? What is the
nature of these minor improvements (are they behavior changes or just
cleanups)? (Remember, the reader hasn't seen the patch yet.)
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -25,17 +25,22 @@ static char *get_encoding(const char *message);
>
> void remove_sequencer_state(int aggressive)
> {
> + const char *seq_dir = git_path(SEQ_DIR);
> + const char *seq_old_dir = git_path(SEQ_OLD_DIR);
> + const char *cherry_pick_head = git_path("CHERRY_PICK_HEAD");
If there were just two more like this, the behavior would change
completely. Scary. Are these temporary variables needed?
^ permalink raw reply
* Re: [PATCH 1/5] sequencer: factor code out of revert builtin
From: Jonathan Nieder @ 2011-11-06 0:12 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <1320510586-3940-2-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Start building the generalized sequencer by moving code from revert.c
> into sequencer.c and sequencer.h. Make the builtin responsible only
> for command-line parsing, and expose a new sequencer_pick_revisions()
> to do the actual work of sequencing commits.
>
> This is intended to be almost a pure code movement patch with no
> functional changes. Check with:
Do I understand correctly that the purpose of this patch is to expose
some functions through the "sequencer.h" API, which patches later in
the series will use? Which functions? What is this generalized
sequencer which we are starting to build? Why should I be happy about
(or care about, for that matter) code having moved from one source
file to another?
Rule of thumb for commit messages: after reading a commit message, I
should be able to predict what the patch will do, without reading the
patch.
I am guessing the above description started sane and then went through
a few revisions without a person reading it all the way through again.
Please consider just rewriting it.
[...]
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -1,19 +1,9 @@
> #include "cache.h"
> #include "builtin.h"
> -#include "object.h"
> -#include "commit.h"
> -#include "tag.h"
> -#include "run-command.h"
> -#include "exec_cmd.h"
> -#include "utf8.h"
> #include "parse-options.h"
> -#include "cache-tree.h"
> #include "diff.h"
> #include "revision.h"
> #include "rerere.h"
> -#include "merge-recursive.h"
> -#include "refs.h"
> -#include "dir.h"
> #include "sequencer.h"
Hoorah!
[snipping lots of deletion of code from builtin/revert.c]
> @@ -1011,7 +194,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
> opts.action = REPLAY_REVERT;
> git_config(git_default_config, NULL);
> parse_args(argc, argv, &opts);
> - res = pick_revisions(&opts);
> + res = sequencer_pick_revisions(&opts);
The new sequencer_pick_revisions is just a new name for the old
pick_revisions. Sane, but probably worth mentioning in the log
message.
[...]
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1,7 +1,27 @@
> #include "cache.h"
> +#include "object.h"
> +#include "commit.h"
> +#include "tag.h"
> +#include "run-command.h"
> +#include "exec_cmd.h"
> +#include "utf8.h"
> +#include "cache-tree.h"
> +#include "diff.h"
> +#include "revision.h"
> +#include "rerere.h"
> +#include "merge-recursive.h"
> +#include "refs.h"
> -#include "sequencer.h"
> -#include "strbuf.h"
> #include "dir.h"
> +#include "sequencer.h"
Why did sequencer.h move to after dir.h? Wow, we use a lot of headers
here --- I wonder if there are some pieces that could be split out
(that's not due to your patch, though).
[...]
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -8,6 +8,30 @@
> #define SEQ_OPTS_FILE "sequencer/opts"
>
> enum replay_action { REPLAY_REVERT, REPLAY_PICK };
> +enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
> +
> +struct replay_opts {
[...]
> @@ -25,4 +49,6 @@ struct replay_insn_list {
> */
> void remove_sequencer_state(int aggressive);
>
> +int sequencer_pick_revisions(struct replay_opts *opts);
Ah, so this moves most of the logic of "git cherry-pick" to the sequencer
but the only new API that needs to be exposed is pick_revisions(). The
calling sequence looks like this:
memset(&opts, o, sizeof(opts));
opts.action = REPLAY_PICK;
opts.revs = xmalloc(sizeof(*opts.revs));
init_revisions(opts.revs);
add_pending_object / setup_revisions / etc
sequencer_pick_revisions(&opts);
The small exposed interface makes this a relatively uninvasive patch,
and the immediate advantage is that we plan to reuse some of the
functionality used in pick_revisions() in other, new APIs to be used
by commands other than cherry-pick. No functional change yet intended.
Except for the commit message, looks reasonable (though I haven't
tried the "git blame" magic to check the code movement part). Thanks.
^ permalink raw reply
* Re: [PATCH] fsck: print progress
From: Junio C Hamano @ 2011-11-05 23:59 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Nguyễn Thái Ngọc Duy, git, Jeff King
In-Reply-To: <4EB4EB70.40801@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> What progress isn't shown? How about
>
> If --verbose is used with --progress the progress status
> will not be shown.
When I ask for verbose output, I do not get progress eye-candy?
Why?
^ permalink raw reply
* Re: How do I get a squashed diff for review
From: Junio C Hamano @ 2011-11-05 23:53 UTC (permalink / raw)
To: Alexander Usov; +Cc: David Aguilar, git, Roland Kaufmann
In-Reply-To: <CAH_EFyYUja4cKY5YM4Uqn-bnQZCnhnJCNsxGsUitL+SSqj9qxQ@mail.gmail.com>
Alexander Usov <a.s.usov@gmail.com> writes:
> Consider the following history:
>
> master: A---B---D---F
> \ \
> branch: .-C--E--G
>
> ...
> - "git diff D..branch" would do a trick, but I'm not sure how to
> correctly determine
> D (if I'm to write a script).
D is the merge-base between F and G. So "git diff $(git merge-base F G) G"
would compare D and G.
Modern git lets you write it as "git diff F...G" as this is a fairly
useful short-hand.
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-05 23:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
Andrew Morton, linux-ide, LKML
In-Reply-To: <CA+55aFy0gA0ROSyE03h6Lw0zn4B4j-oEFBmffOcWs6NfyYy8JA@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> So I'd really like some way to not see it.
>
> Ted suggested a NUL character in the commit message in front of the
> "hidden content". What do you think?
You do not have to resort to NUL; we could just stuff whatever you do not
need to see but needs to be left *intact* in the new header fields just
like the embedded GPG signatures are stored in signed commits.
By the time the integrator is presented the merge commit template, we
would have:
1. The merge title (e.g. "Merge tag for-linus of git://.../rusty.git/");
2. Payload of the signed tag (or just "annotated tag"), which is used to
convey meaningful topic description from the lieutenant;
3. The signature in the tag, if the tag is not just merely annotated, but
is signed;
4. The output from GPG verification of the above (only when 3. is
available); and
5. The traditional "merge summary", if merge.log is enabled.
The 10-patch series I sent earlier appends 2 and 3 with "tag:" prefix and
4 with "# " prefix in the commit log template, but it does not have to be
that way. We could arrange things so that we put only 1, 2, 4 (still with
"# " prefix because this is meant to help you verify the authenticity, not
for later third-party audit, and to be stripped away with stripspace
before the commit is made) and 5 in the commit log template, and the
original signed tag contents (only when the tag is signed, not merely
annotated) in a separate file MERGE_SIG in $GIT_DIR/ next to MERGE_MSG,
and teach "git commit" to pick it up and stuff it in a new header field.
That way, the integrator can use the message 2 for the commit log message
and is free to typofix it, without breaking later third-party audit which
would use what is taken literally from the signed tag and stored in the
new header field, because the integrator's editor would never touch the
latter.
^ permalink raw reply
* [GITTOGATHER] Amsterdam git hackathon a great success
From: Sverre Rabbelier @ 2011-11-05 23:43 UTC (permalink / raw)
To: Git List
Cc: Ævar Arnfjörð Bjarmason, Eric Herman,
Fernando Vezzosi
Heya,
Todays GitTogather (in Amsterdam) organized by Ævar and kindly hosted
by Eric was a great success, resulting in no less than seven patch
series being sent to the mailing list (including a re-roll of the
fe-fixes series)!
Many thanks to all involved for a great day :)
http://thread.gmane.org/gmane.comp.version-control.git/184849
http://thread.gmane.org/gmane.comp.version-control.git/184872
http://thread.gmane.org/gmane.comp.version-control.git/184848
http://thread.gmane.org/gmane.comp.version-control.git/184853
http://thread.gmane.org/gmane.comp.version-control.git/184847
http://thread.gmane.org/gmane.comp.version-control.git/184868
http://thread.gmane.org/gmane.comp.version-control.git/184874
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 0/5] Sequencer: working around historical mistakes
From: Jonathan Nieder @ 2011-11-05 23:43 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <1320510586-3940-1-git-send-email-artagnon@gmail.com>
Hey,
Ramkumar Ramachandra wrote:
> As described in the discussion following $gmane/179304/focus=179383,
> we have decided to handle historical hacks in the sequencer itself.
> This series that follows is one step in the right direction.
I'm not sure what the above means. But let's see what the patches
say. :)
[...]
> 2. This series depends on rr/revert-cherry-pick, but doesn't apply to
> the current 'next'- sorry, rebasing is a massive pita due to 1/5.
Shouldn't it be based against rr/revert-cherry-pick, rather than
"next" which is more of a moving target?
Thanks,
Jonathan
^ permalink raw reply
* [PATCH 3/3] fast-export: output reset command for commandline revs
From: Sverre Rabbelier @ 2011-11-05 23:23 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Nieder, Jeff King, Git List,
Daniel Barkalow, Ramkumar
Cc: Sverre Rabbelier
In-Reply-To: <1320535407-4933-1-git-send-email-srabbelier@gmail.com>
When a revision is specified on the commandline we explicitly output
a 'reset' command for it if it was not handled already. This allows
for example the remote-helper protocol to use fast-export to create
branches that point to a commit that has already been exported.
Initial-patch-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Most of the hard work for this patch was done by Dscho. The rest of
it was basically me applying the technique used by jch in c3502fa
(25-08-2011 do not include sibling history in --ancestry-path).
The if statement dealing with tag_of_filtered_mode is not as
elegant as either me or Dscho would have liked, but we couldn't
find a better way to determine if a ref is a tag at this point
in the code.
Additionally, the elem->whence != REV_CMD_RIGHT case should really
check if REV_CMD_RIGHT_REF, but as this is not provided by the
ref_info structure this is left as is. A result of this is that
incorrect input will result in incorrect output, rather than an
error message. That is: `git fast-export a..<sha1>` will
incorrectly generate a `reset <sha1>` statement in the fast-export
stream.
The dwim_ref bit is a double work (it has already been done by the
caller of this function), but I decided it would be more work to
pass this information along than to recompute it for the few
commandline refs that were relevant.
builtin/fast-export.c | 31 +++++++++++++++++++++++++++++--
t/t9350-fast-export.sh | 2 +-
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c4c4391..bcfec38 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -18,6 +18,8 @@
#include "parse-options.h"
#include "quote.h"
+#define REF_HANDLED (ALL_REV_FLAGS + 1)
+
static const char *fast_export_usage[] = {
"git fast-export [rev-list-opts]",
NULL
@@ -541,10 +543,34 @@ static void handle_reset(const char *name, struct object *object)
sha1_to_hex(object->sha1));
}
-static void handle_tags_and_duplicates(struct string_list *extra_refs)
+static void handle_tags_and_duplicates(struct rev_info *revs, struct string_list *extra_refs)
{
int i;
+ /* even if no commits were exported, we need to export the ref */
+ for (i = 0; i < revs->cmdline.nr; i++) {
+ struct rev_cmdline_entry *elem = &revs->cmdline.rev[i];
+
+ if (elem->flags & UNINTERESTING)
+ continue;
+
+ if (elem->whence != REV_CMD_REV && elem->whence != REV_CMD_RIGHT)
+ continue;
+
+ char *full_name;
+ dwim_ref(elem->name, strlen(elem->name), elem->item->sha1, &full_name);
+
+ if (!prefixcmp(full_name, "refs/tags/") &&
+ (tag_of_filtered_mode != REWRITE ||
+ !get_object_mark(elem->item)))
+ continue;
+
+ if (!(elem->flags & REF_HANDLED)) {
+ handle_reset(full_name, elem->item);
+ elem->flags |= REF_HANDLED;
+ }
+ }
+
for (i = extra_refs->nr - 1; i >= 0; i--) {
const char *name = extra_refs->items[i].string;
struct object *object = extra_refs->items[i].util;
@@ -698,11 +724,12 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
}
else {
handle_commit(commit, &revs);
+ commit->object.flags |= REF_HANDLED;
handle_tail(&commits, &revs);
}
}
- handle_tags_and_duplicates(&extra_refs);
+ handle_tags_and_duplicates(&revs, &extra_refs);
if (export_filename)
export_marks(export_filename);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 74914dc..ea7dc21 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -446,7 +446,7 @@ from $(git rev-parse master)
EOF
-test_expect_failure 'refs are updated even if no commits need to be exported' '
+test_expect_success 'refs are updated even if no commits need to be exported' '
git fast-export master..master > actual &&
test_cmp expected actual
'
--
1.7.8.rc0.36.g67522.dirty
^ permalink raw reply related
* [PATCH 1/3] t9350: point out that refs are not updated correctly
From: Sverre Rabbelier @ 2011-11-05 23:23 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Nieder, Jeff King, Git List,
Daniel Barkalow, Ramkumar
Cc: Sverre Rabbelier
In-Reply-To: <1320535407-4933-1-git-send-email-srabbelier@gmail.com>
This happens only when the corresponding commits are not exported in
the current fast-export run. This can happen either when the relevant
commit is already marked, or when the commit is explicitly marked
as UNINTERESTING with a negative ref by another argument.
This breaks fast-export based remote helpers, as they use marks
files to store which commits have already been seen. The call graph
is something as follows:
$ # push master to remote repo
$ git fast-export --{im,ex}port-marks=marksfile master
$ # make a commit on master and push it to remote
$ git fast-export --{im,ex}port-marks=marksfile master
$ # run `git branch foo` and push it to remote
$ git fast-export --{im,ex}port-marks=marksfile foo
When fast-export imports the marksfile and sees that all commits in
foo are marked as UNINTERESTING (they have already been exported
while pushing master), it exits without doing anything. However,
what we want is for it to reset 'foo' to the already-exported commit.
Either way demonstrates the problem, and since this is the most
succint way to demonstrate the problem it is implemented by passing
master..master on the commandline.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
t/t9350-fast-export.sh | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 950d0ff..74914dc 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -440,4 +440,15 @@ test_expect_success 'fast-export quotes pathnames' '
)
'
+cat > expected << EOF
+reset refs/heads/master
+from $(git rev-parse master)
+
+EOF
+
+test_expect_failure 'refs are updated even if no commits need to be exported' '
+ git fast-export master..master > actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.7.8.rc0.36.g67522.dirty
^ permalink raw reply related
* [PATCH 2/3] fast-export: do not refer to non-existing marks
From: Sverre Rabbelier @ 2011-11-05 23:23 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Nieder, Jeff King, Git List,
Daniel Barkalow, Ramkumar
Cc: Sverre Rabbelier
In-Reply-To: <1320535407-4933-1-git-send-email-srabbelier@gmail.com>
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
When calling `git fast-export a..a b` when a and b refer to the same
commit, nothing would be exported, and an incorrect reset line would
be printed for b ('from :0').
Extract a handle_reset function that deals with this, which can then
be re-used in a later commit.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
builtin/fast-export.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9836e6b..c4c4391 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -529,9 +529,20 @@ static void get_tags_and_duplicates(struct object_array *pending,
}
}
+static void handle_reset(const char *name, struct object *object)
+{
+ int mark = get_object_mark(object);
+
+ if (mark)
+ printf("reset %s\nfrom :%d\n\n", name,
+ get_object_mark(object));
+ else
+ printf("reset %s\nfrom %s\n\n", name,
+ sha1_to_hex(object->sha1));
+}
+
static void handle_tags_and_duplicates(struct string_list *extra_refs)
{
- struct commit *commit;
int i;
for (i = extra_refs->nr - 1; i >= 0; i--) {
@@ -543,9 +554,7 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)
break;
case OBJ_COMMIT:
/* create refs pointing to already seen commits */
- commit = (struct commit *)object;
- printf("reset %s\nfrom :%d\n\n", name,
- get_object_mark(&commit->object));
+ handle_reset(name, object);
show_progress();
break;
}
--
1.7.8.rc0.36.g67522.dirty
^ permalink raw reply related
* [PATCH 0/3] fast-export fixes
From: Sverre Rabbelier @ 2011-11-05 23:23 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Nieder, Jeff King, Git List,
Daniel Barkalow, Ramkumar
Cc: Sverre Rabbelier
Dscho and I worked on this patch series during a mini-hackathon in
late July, but Junio held the series back since he saw a more elegant
way to tackle the problem that would pave the way to solve a problem
he was having. Since then I've had very little time to work on git,
so I was very glad to have the chance to work on this during another
mini-hackathon in Amsterdam today.
I've used the new rev_info mechanism Junio introduced, and while I
can't say I completely understand how the tag_of_filtered_mode bit
works, I'm happy to say that all the tests pass now :).
Johannes Schindelin (2):
fast-export: do not refer to non-existing marks
setup_revisions: remember whether a ref was positive or not
Sverre Rabbelier (1):
t9350: point out that refs are not updated correctly
builtin/fast-export.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
t/t9350-fast-export.sh | 11 +++++++++++
2 files changed, 53 insertions(+), 6 deletions(-)
--
1.7.8.rc0.36.g67522.dirty
^ permalink raw reply
* Re: [PATCH] fsck: print progress
From: Stephen Boyd @ 2011-11-05 19:15 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <CACsJy8CEh6x1O0AHpiErZ_+T2hGwjRZofygmsgVX8_WXV7uDTA@mail.gmail.com>
On 11/05/2011 02:02 AM, Nguyen Thai Ngoc Duy wrote:
>
> "-q" in fetch-options.txt can be converted to "--no-progress or
> --verbose". How about this?
>
> --progress::
> --no-progress::
> Progress status is reported on the standard error stream by
> default when it is attached to a terminal, unless
> --no-progress or --verbose is specified. --progress forces
> progress status even if the standard error stream is not
> directed to a terminal.
Looks good.
^ permalink raw reply
* [PATCH] Introduce commit.verbose config option
From: Fernando Vezzosi @ 2011-11-05 18:07 UTC (permalink / raw)
To: git
Enabling commit.verbose will make git commit behave as if --verbose was
passed on the command line.
Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
---
Documentation/config.txt | 3 +++
builtin/commit.c | 4 ++++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..6826788 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -832,6 +832,9 @@ commit.template::
"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
specified user's home directory.
+commit.verbose::
+ A boolean to enable verbose mode like the --verbose flag does.
+
include::diff-config.txt[]
difftool.<tool>.path::
diff --git a/builtin/commit.c b/builtin/commit.c
index c46f2d1..c2a1f3d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1331,6 +1331,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
+ if (!strcmp(k, "commit.verbose")) {
+ verbose = git_config_bool(k, v);
+ return 0;
+ }
return git_status_config(k, v, s);
}
--
1.7.7.1
^ permalink raw reply related
* Re: How do I get a squashed diff for review
From: Alexander Usov @ 2011-11-05 17:56 UTC (permalink / raw)
To: David Aguilar; +Cc: git, Roland Kaufmann
In-Reply-To: <20111105091514.GA97860@gmail.com>
On 5 November 2011 09:15, David Aguilar <davvid@gmail.com> wrote:
> On Fri, Nov 04, 2011 at 07:15:01PM +0000, Alexander Usov wrote:
>> Hi,
>>
>> However if the feature branch happened to be long-lived and had
>> mainline merged into it it's not going to work -- the
>> resulting diff would contain changes from the merge. The way we are
>> doing things now is to merge master into it
>> once more and then diff, however this is somewhat cumbersome. Is there
>> easier way to do it?
>
> "git diff A...B" is equivalent to "git diff <merge-base A B> B".
> The merge-base can be found with "git merge-base A B"
> and is simply the common ancestor of A and B.
>
> Diffing against the merge base (which doesn't contain the merged
> work done in master) is why you're seeing the merges in the diff.
>
> It sounds like you want the simpler form of "diff" which doesn't
> do any merge-base calculation.
>
> e.g. "git diff A B" and its synonymn "git diff A..B".
Just diffing 2 revisions (or trees) won't do the trick. Let me try to explain
what I'm trying to achieve.
Consider the following history:
master: A---B---D---F
\ \
branch: .-C--E--G
Now I want to review the changes made in the branch prior to merging it.
What I essentially want to be included in the diff are changes committed in
C & G and conflic resolution done in E (if any).
There are few ways that I know of to achieve it:
- use "git log -p branch ^master " to get a sequence of patches and try to
feed them into combinediff tool (part of the diffutils package). This
will require
some scripting and I'm not really sure if combinediff would work with
git patches.
- "git diff D..branch" would do a trick, but I'm not sure how to
correctly determine
D (if I'm to write a script). This would be the last (in topological
order) commit which is
reachable from both master & branch. Any suggestions on it?
>> And while we are on the topic -- is there a tool for git similar to "bzr qdiff"?
>> It's a simple graphical diff viewer with 2 nice features -- it shows
>> complete diff (of multiple files) in a single window and
>> has a checkbox to switch between diff-only & full-text modes.
>> I have seen difftool, but it seems to work on per-file basis, and
>> something like "vi <(git diff ...)" lacks the easy way to
>> switch into full-text mode.
>
> difftool is a wrapper around specialized diff tools, so the
> ability to switch from diff to full view is tool-dependent.
>
> A contrib "git-dirdiff" script was posted to the list recently.
> It builds upon diff tools that can diff directory trees.
>
> http://thread.gmane.org/gmane.comp.version-control.git/184528
>
> There may be a newer version of this script, too. Roland would
> know for sure...
Thanks. Will have a more carefull look at various tools & see if I can
figure something out.
--
Best regards,
Alexander.
^ permalink raw reply
* [PATCH] git-p4: ignore apple filetype
From: Pete Wyckoff @ 2011-11-05 17:36 UTC (permalink / raw)
To: Git Mailing List; +Cc: Michael Wookey, Vitor Antunes, Luke Diamand
In-Reply-To: <20111104183957.GB18517@padd.com>
Revert 97a21ca (git-p4: stop ignoring apple filetype, 2011-10-16)
and add a test case.
Reported-by: Michael Wookey <michaelwookey@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
This is mostly a revert, but the test moves down a bit to be near
a similar clause for utf16. Adding a big comment and test case
hopefully keeps this code in place in the future.
Michael: if you're willing to test this, I'd appreciate it. In
fact, running all the git-p4 unit tests on Mac would be great
if you have a p4d:
mac$ ( cd t ; make t98* )
contrib/fast-import/git-p4 | 13 +++++++++++++
t/t9802-git-p4-filetype.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index f885d70..b975d67 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1318,6 +1318,19 @@ class P4Sync(Command, P4UserMap):
text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
contents = [ text ]
+ if type_base == "apple":
+ # Apple filetype files will be streamed as a concatenation of
+ # its appledouble header and the contents. This is useless
+ # on both macs and non-macs. If using "print -q -o xx", it
+ # will create "xx" with the data, and "%xx" with the header.
+ # This is also not very useful.
+ #
+ # Ideally, someday, this script can learn how to generate
+ # appledouble files directly and import those to git, but
+ # non-mac machines can never find a use for apple filetype.
+ print "\nIgnoring apple filetype file %s" % file['depotFile']
+ return
+
# Perhaps windows wants unicode, utf16 newlines translated too;
# but this is not doing it.
if self.isWindows and type_base == "text":
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 3b358ef..992bb8c 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -101,6 +101,37 @@ test_expect_success 'keyword file test' '
)
'
+build_gendouble() {
+ cat >gendouble.py <<-\EOF
+ import sys
+ import struct
+ import array
+
+ s = array.array("c", '\0' * 26)
+ struct.pack_into(">L", s, 0, 0x00051607) # AppleDouble
+ struct.pack_into(">L", s, 4, 0x00020000) # version 2
+ s.tofile(sys.stdout)
+ EOF
+}
+
+test_expect_success 'ignore apple' '
+ test_when_finished rm -f gendouble.py &&
+ build_gendouble &&
+ (
+ cd "$cli" &&
+ test-genrandom apple 1024 >double.png &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/gendouble.py" >%double.png &&
+ p4 add -t apple double.png &&
+ p4 submit -d appledouble
+ ) &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+ test ! -f double.png
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
1.7.7.345.g88d3c
^ permalink raw reply related
* [PATCH 2/2] t/t7508-status.sh: use test_i18ncmp
From: Ævar Arnfjörð Bjarmason @ 2011-11-05 17:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jon Seymour,
Ævar Arnfjörð Bjarmason
In-Reply-To: <1320514123-18842-1-git-send-email-avarab@gmail.com>
Change a i18n-specific comparison in t/t7508-status.sh to use
test_i18ncmp instead. This was introduced in v1.7.6.3~11^2 and has
been broken under GETTEXT_POISON=YesPlease since.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t7508-status.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 905255a..fc57b13 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -189,7 +189,7 @@ test_expect_success 'status with gitignore' '
# untracked
EOF
git status --ignored >output &&
- test_cmp expect output
+ test_i18ncmp expect output
'
test_expect_success 'status with gitignore (nothing untracked)' '
@@ -247,7 +247,7 @@ test_expect_success 'status with gitignore (nothing untracked)' '
# untracked
EOF
git status --ignored >output &&
- test_cmp expect output
+ test_i18ncmp expect output
'
rm -f .gitignore
--
1.7.7
^ permalink raw reply related
* [PATCH 1/2] t/t6030-bisect-porcelain.sh: use test_i18ngrep
From: Ævar Arnfjörð Bjarmason @ 2011-11-05 17:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jon Seymour,
Ævar Arnfjörð Bjarmason
In-Reply-To: <1320514123-18842-1-git-send-email-avarab@gmail.com>
Change a i18n-specific grep in t/t6030-bisect-porcelain.sh to use
test_i18ngrep instead. This was introduced in v1.7.7.2~5^2~11 and has
been broken under GETTEXT_POISON=YesPlease since.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t6030-bisect-porcelain.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index c6f1f9f..691e4a4 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
cp .git/BISECT_START saved &&
test_must_fail git bisect start $HASH4 foo -- &&
git branch > branch.output &&
- grep "* (no branch)" branch.output > /dev/null &&
+ test_i18ngrep "* (no branch)" branch.output > /dev/null &&
test_cmp saved .git/BISECT_START
'
test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
--
1.7.7
^ permalink raw reply related
* [PATCH 0/2] i18n: fix GETTEXT_POISON=YesPlease breakages
From: Ævar Arnfjörð Bjarmason @ 2011-11-05 17:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jon Seymour,
Ævar Arnfjörð Bjarmason
This fixes things broken in the test suite under
GETTEXT_POISON=YesPlease.
Ævar Arnfjörð Bjarmason (2):
t/t6030-bisect-porcelain.sh: use test_i18ngrep
t/t7508-status.sh: use test_i18ncmp
t/t6030-bisect-porcelain.sh | 2 +-
t/t7508-status.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
--
1.7.7
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-05 16:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
Andrew Morton, linux-ide, LKML
In-Reply-To: <7v1utn9it8.fsf@alter.siamese.dyndns.org>
On Fri, Nov 4, 2011 at 11:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> About the ugliness of the merge commit log messages, you have already
> learned to ignore them with "log --no-merges" ;-)
Absolutely not. I look at merges all the time. I never use
"--no-merges" except when I'm doing certain statistics (ie "How many
real changes do we have") or when I do release files.
But I actually think it's important that people write *good* merge
messages. I've berated some people for it when they just have
Merge branch 'origin'
in their commit message, because I think a merge commit should say why
it happened or what it brought in.
> and the material the
> patch series I sent out adds are at the end, so "/^commit.*$" in less
> would hopefully work well enough in "log --no-merges" as well.
I agree that being at the end helps, but I do a lot of "git log
ORIG_HEAD.." etc, and I don't do a lot of "/^commit" searching.
The "/commit" thing I do tends to be because I do "git log -p" to see
patches, but at the same time am not going to read through
everything..
So I'd really like some way to not see it.
Ted suggested a NUL character in the commit message in front of the
"hidden content". What do you think?
Linus
^ permalink raw reply
* Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX
From: Johannes Sixt @ 2011-11-05 16:34 UTC (permalink / raw)
To: Nick Alcock; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <4EB5583E.2030306@kdbg.org>
Am 05.11.2011 16:37, schrieb Johannes Sixt:
> Commit f696543d (Add strtoimax() compatibility function) introduced an
> implementation of the function, but forgot to add a declaration.
On second thought, I'm puzzled: Without this patch and without noticing
the warning that strtoimax() was not declared, I had built with
NO_STRTOUMAX on MinGW before, and the build succeeded. This means that
even though MinGW's headers are not C99, we must have pulled in function
strtoimax() from somewhere. I'll investigate later this weekend.
Anyway, this patch does not just add a declaration for the function, but
also redirects strtoimax to gitstrtoimax, which is a bit more than the
commit message claims. Without this patch, topic na/strtoimax should not
build on a non-C99 environment. Can you verify this claim?
-- Hannes
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> git-compat-util.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index feb6f8e..4efef46 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -354,6 +354,8 @@ extern size_t gitstrlcpy(char *, const char *, size_t);
> #ifdef NO_STRTOUMAX
> #define strtoumax gitstrtoumax
> extern uintmax_t gitstrtoumax(const char *, char **, int);
> +#define strtoimax gitstrtoimax
> +extern intmax_t gitstrtoimax(const char *, char **, int);
> #endif
>
> #ifdef NO_STRTOK_R
^ permalink raw reply
* [PATCH 5/5] sequencer: revert d3f4628e
From: Ramkumar Ramachandra @ 2011-11-05 16:29 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1320510586-3940-1-git-send-email-artagnon@gmail.com>
Revert d3f4628e (revert: Remove sequencer state when no commits are
pending, 2011-06-06), because this is not the right approach. Instead
of increasing coupling between the sequencer and 'git commit', a
unified '--continue' that invokes 'git commit' on behalf of the
end-user is preferred.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 12 +-----------
t/t3510-cherry-pick-sequence.sh | 24 ------------------------
2 files changed, 1 insertions(+), 35 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 6762ceb..7caa550 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -758,18 +758,8 @@ static int pick_commits(struct replay_insn_list *todo_list,
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur);
res = do_pick_commit(cur->operand, cur->action, opts);
- 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);
+ if (res)
return res;
- }
}
/*
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 4b12244..b30f13a 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -85,30 +85,6 @@ 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_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
--
1.7.6.351.gb35ac.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