* [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Andrew Myrick @ 2010-01-07 0:25 UTC (permalink / raw)
To: git; +Cc: sam, normalperson, Andrew Myrick
In-Reply-To: <E10FB265-0C47-44C7-9347-687A9F447603@apple.com>
Update git-svn to ignore commits that do not change the tree when it is
deciding if an svn merge ticket represents a real branch merge or just a
cherry-pick.
Consider the following integration model in the svn repository:
F---G branch1
/ \
D tag1 \ E tag2
/ \ /
A---B C trunk
branch1 is merged to trunk in commit C.
With this patch, git-svn will correctly identify branch1 as a proper merge
parent, instead of incorrectly ignoring it as a cherry-pick.
Signed-off-by: Andrew Myrick <amyrick@apple.com>
---
git-svn.perl | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 650c9e5..947184a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3052,12 +3052,36 @@ sub check_cherry_pick {
for my $range ( @ranges ) {
delete @commits{_rev_list($range)};
}
+ for my $commit (keys %commits) {
+ if (has_no_changes($commit)) {
+ delete $commits{$commit};
+ }
+ }
return (keys %commits);
}
+sub has_no_changes {
+ my $commit = shift;
+
+ my @revs = split / /, command_oneline(
+ qw(rev-list --parents -1 -m), $commit);
+
+ # Commits with no parents, e.g. the start of a partial branch,
+ # have changes by definition.
+ return 1 if (@revs < 2);
+
+ # Commits with multiple parents, e.g a merge, have no changes
+ # by definition.
+ return 0 if (@revs > 2);
+
+ return (command_oneline("rev-parse", "$commit^{tree}") eq
+ command_oneline("rev-parse", "$commit~1^{tree}"));
+}
+
BEGIN {
memoize 'lookup_svn_merge';
memoize 'check_cherry_pick';
+ memoize 'has_no_changes';
}
sub parents_exclude {
--
1.6.6.2.g18c9a
^ permalink raw reply related
* [PATCH 2/2] git-svn: handle merge-base failures
From: Andrew Myrick @ 2010-01-07 0:25 UTC (permalink / raw)
To: git; +Cc: sam, normalperson, Andrew Myrick
In-Reply-To: <1262823922-3415-1-git-send-email-amyrick@apple.com>
Change git-svn to warn and continue when merge-base fails while processing svn
merge tickets.
merge-base can fail when a partial branch is created and merged back to trunk
in svn, because it cannot find a common ancestor between the partial branch and
trunk.
Signed-off-by: Andrew Myrick <amyrick@apple.com>
---
git-svn.perl | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 947184a..1f201e4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3158,10 +3158,21 @@ sub find_extra_svn_parents {
my $ranges = $ranges{$merge_tip};
# check out 'new' tips
- my $merge_base = command_oneline(
- "merge-base",
- @$parents, $merge_tip,
- );
+ my $merge_base;
+ eval {
+ $merge_base = command_oneline(
+ "merge-base",
+ @$parents, $merge_tip,
+ );
+ };
+ if ($@) {
+ die "An error occurred during merge-base"
+ unless $@->isa("Git::Error::Command");
+
+ warn "W: Cannot find common ancestor between ".
+ "@$parents and $merge_tip. Ignoring merge info.\n";
+ next;
+ }
# double check that there are no missing non-merge commits
my (@incomplete) = check_cherry_pick(
--
1.6.6.2.g18c9a
^ permalink raw reply related
* 1.6.1.3: git merge --no-commit ... DID commit
From: layer @ 2010-01-07 1:41 UTC (permalink / raw)
To: git
quadra% git merge --no-commit duane-acl82/acl82
Updating 621f935..a94f7fc
Fast forward
ChangeLog | 23 ++++++++++++++
src/c/fio.c | 2 +-
src/c/mon1.c | 6 ++-
src/code/debug.cl | 10 +++++-
src/code/lldb.cl | 75 +++++++++++++++++++++++++++++++++++++---------
src/code/osi-alpha.cl | 33 ++++++++++++++++++++
src/code/osi-alpha64.cl | 33 ++++++++++++++++++++
src/code/x/disx86-64.cl | 4 ++
src/rs/vrlinux.cl | 3 +-
src/rs/xrlinux64.cl | 30 +++++++++++++++----
src/rs/xrmacosx64.cl | 44 ++++++++++++++++++++++++----
src/rs/xrms64.cl | 28 ++++++++++++++---
src/rs/xrsol64.cl | 28 ++++++++++++++---
13 files changed, 276 insertions(+), 43 deletions(-)
quadra% git status
# On branch acl82
# Your branch is ahead of 'origin/acl82' by 2 commits.
#
nothing to commit (working directory clean)
quadra%
Has this been fixed in a later version?
^ permalink raw reply
* Re: A question about changing remote repo name
From: Russell Steicke @ 2010-01-07 1:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Dongas, git
In-Reply-To: <alpine.DEB.1.00.1001060123280.4985@pacific.mpi-cbg.de>
On Wed, Jan 6, 2010 at 8:27 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> For everybody reading this thread: DON'T!
>
> Russel, it would be better if you knew about the mechanism before
> making other people believe that it is safe to delete a file that is _not_
> a cache, but _replaces_ the existing refs.
Thank you for explaining that. I apologise for the misleading information.
I have seen refs in packed-refs as duplicates of those in the refs/
tree. How would that come about?
--
Virus found in this message.
^ permalink raw reply
* Re: 1.6.1.3: git merge --no-commit ... DID commit
From: Junio C Hamano @ 2010-01-07 2:13 UTC (permalink / raw)
To: layer; +Cc: git
In-Reply-To: <11785.1262828518@relay.known.net>
layer <layer@known.net> writes:
> quadra% git merge --no-commit duane-acl82/acl82
> Updating 621f935..a94f7fc
> Fast forward
> ...
> quadra% git status
> # On branch acl82
> # Your branch is ahead of 'origin/acl82' by 2 commits.
> #
> nothing to commit (working directory clean)
> ...
> Has this been fixed in a later version?
In short, there is nothing to fix. You asked it not to create a commit,
and the merge was a fast-forward; there was no new commit created.
When a real merge is involved, e.g. you have this history:
x---x---x---x---B duane-acl82/acl82
/
---O---o---o---o---A HEAD
telling "--no-commit" to merge creates the state to be committed in your
work tree and the index, notes the fact that the next "git commit" will
record a merge between A and B, and stops. Hence, "git diff HEAD" will
show the damange merging duane-acl82/acl82 will cause to your current
branch, and then you can "git commit" to record the merge to result in
this history:
x---x---x---x---B duane-acl82/acl82
/ \
---O---o---o---o---A---* HEAD
A --no-commit merge followed by committing on your own will result in a
history of the same shape as "merge" without --no-commit will create.
Think what you want to happen if you started from this history:
x---x---x---x---B duane-acl82/acl82
/
---A HEAD
If "merge --no-commit" left HEAD at A but updated the index and the work
tree to the result of the merge, which would be the same as the tree
recorded by commit B, and prepared to record a merge commit between A and
B, then next "git commit" will not create a history of the same shape as
you would normally get from "merge" without --no-commit, which is:
HEAD
x---x---x---x---B duane-acl82/acl82
/
---A
Instead, you will end up with a history of this shape, with one useless
merge commit:
x---x---x---x---B duane-acl82/acl82
/ \
---A-------------------* HEAD
That is why "merge --no-commit" will fast-forward.
If you really want to do this, there is a way to create a history with
such a shape ("git merge --no-ff") but by default it is not recommended
and you need to explicitly ask for it (or configure). It can also be used
together with --no-commit option.
Often people claim that they want to review before actually merging, but
it is much better to get in the habit of running "git merge topic" first
then inspecting "git diff ORIG_HEAD^ after the fact. If the result is
undesirable, you can always "git reset --hard ORIG_HEAD" it away. The
reason it is better is that this will work regardless of the kind of merge
you would end up with; you can reset away a fast-forward using ORIG_HEAD.
Another technique that may be worth learning is to do "git diff ...topic"
before running a merge (notice three dots).
^ permalink raw reply
* Re: [PATCH git-gui 5/5] git-gui/Makefile: consolidate .FORCE-* targets
From: Shawn O. Pearce @ 2010-01-07 2:20 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano
In-Reply-To: <20100106081638.GF7298@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> wrote:
> Providing multiple targets to force a rebuild is unnecessary
> complication.
>
> Avoid using a name that could conflict with future special
> targets in GNU make (a leading period followed by uppercase
> letters).
>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks, applied.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] git-gui: Add hotkeys for "Unstage from commit" and "Revert changes"
From: Shawn O. Pearce @ 2010-01-07 2:22 UTC (permalink / raw)
To: public_vi; +Cc: git
In-Reply-To: <1262266373-7314-1-git-send-email-public_vi@tut.by>
public_vi@tut.by wrote:
> From: Vitaly _Vi Shukela <public_vi@tut.by>
>
> Signed-off-by: Vitaly _Vi Shukela <public_vi@tut.by>
> ---
> git-gui/git-gui.sh | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
Thanks, applied.
(Ignore my private email from a few minutes ago asking for a repost,
its not necessary.)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/2] t7002: set test prerequisite "external-grep" if supported
From: Junio C Hamano @ 2010-01-07 2:37 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1262608455-4045-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Add another test to set prerequisite "external-grep" if the current
> build supports external grep. This can be used to skip external grep
> only tests on builds that do not support this optimization.
Thanks. We seem to spell our prerequistes in a single-word, all-caps, so
I'll change this new one to EXTGREP in both [1/2] and [2/2].
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2010, #02 draft; Wed, 06)
From: Nanako Shiraishi @ 2010-01-07 3:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvdfenaar.fsf@alter.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.com>
> I am experimenting with ideas to better manage the periodic "What's
> cooking" messages, and here is one of such attempt based on the current
> draft of the upcoming "2010 Jan, issue #02".
>
> This is an incremental update (the full version will follow shortly) that
> shows the changes since the previous issue, and was generated with a
> custom diff driver. One of the things to notice is that the ones that
> only moved across sections (e.g. bg/maint-remote-update-default) without
> any other changes are shown without the list of commits.
>
> ----------------------------------------------------------------
>
> -What's cooking in git.git (Jan 2010, #01; Mon, 04)
> +What's cooking in git.git (Jan 2010, #02 draft; Wed, 06)
>
> --------------------------------------------------
> Born topics
>
> [New Topics]
>
> * jc/maint-1.6.1-checkout-m-custom-merge (2010-01-06) 1 commit
> - checkout -m path: fix recreating conflicts
>
> * jn/makefile (2010-01-06) 4 commits
> - Makefile: consolidate .FORCE-* targets
> - Makefile: learn to generate listings for targets requiring special flags
> - Makefile: use target-specific variable to pass flags to cc
> - Makefile: regenerate assembler listings when asked
>
> --------------------------------------------------
> Moved from [New Topics] to [Cooking]
>
> * da/difftool (2009-12-22) 2 commits
> - - git-difftool: Add '--gui' for selecting a GUI tool
> - - t7800-difftool: Set a bogus tool for use by tests
> + (merged to 'next' on 2010-01-06 at e957395)
> + + git-difftool: Add '--gui' for selecting a GUI tool
> + + t7800-difftool: Set a bogus tool for use by tests
>
> * jh/gitweb-cached (2010-01-03) 4 commits
> - gitweb: Makefile improvements
> - gitweb: Optionally add "git" links in project list page
> - gitweb: Add option to force version match
> - gitweb: Load checking
> +
> +Will merge to 'next', unless I hear objections within a few days.
For what it's worth, I think this new format very easy to spot the differences and much nicer.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: git-cherry-pick problem - directory not touched by commit is marked "added by us"
From: Junio C Hamano @ 2010-01-07 3:30 UTC (permalink / raw)
To: Hakim Cassimally; +Cc: git, Sam Vilain
In-Reply-To: <82cfa8031001060328r21aa8de3s5c2dd5dac005b679@mail.gmail.com>
Hakim Cassimally <hakim.cassimally@gmail.com> writes:
>>> (stable) $ git cherry-pick 301afce1
>>> Finished one cherry-pick.
>>> www/client/css/admin.css: needs merge
>>> <...snip>
>>> www/client/css/admin.css: unmerged (8e7cd850bf40d1a921b1f62ce0945abd374fa55d)
>>> <...snip>
>>> ...
>>> error: Error building trees
>>
> $ git ls-files -s -u www/client/css/admin.css # only staged/unmerged
> 100755 8e7cd850bf40d1a921b1f62ce0945abd374fa55d 2
> www/client/css/admin.css
>
> (i.e. when run after the failed cherry-pick)
>
>> Also take the 'git ls-tree -r HEAD', 'git ls-tree -r 301afce1' and 'git
>> ls-tree -r 301afce1^' output, and grep for the files in question. The
>> answer (or at least a bug triage) should be in the output of those
>> commands.
>
> $ git ls-tree HEAD | grep www/client/css/admin.css
> 100755 blob 8e7cd850bf40d1a921b1f62ce0945abd374fa55d
> www/client/css/admin.css
>
> There's no entry in git ls-tree 301afce1 or 301afce1^1 though. I'd
> guess that's significant, but I don't know what answer it suggests...
>
> (However the file exists in both (stable) and (experimental)...
> and is identical in both).
> ....
So in short:
* commit 301a added a new path bin/upload_module.pl;
* the state you cherry-picked this commit to was clean (i.e. before
running cherry-pick, "git status" reported no local changes to the
index nor to the work tree);
* the commit, the index, and the work tree before running this
cherry-pick had www/client/css/admin.css file, which also existed in
301a and 301a^, but all of these three commits had the same contents.
A simple minded attempt to reproduce this (attached below) by preparing a
commit that adds one new file and attempting to transplant to an unrelated
commit that doesn't have the file didn't work; I have been scratching my
head.
What "cherry-pick" internally does is to run:
git merge-recursive 301a^ -- HEAD 301a
That is, "We are at HEAD; consolidate the change since 301a^ to 301a into
our state, and leave the result in the index and the work tree". Then it
commits the result. One thing to try is to see if this gives the same
kind of breakage.
The only other thing that _may_ be involved is if there are paths that
turned between directories and files on the two histories, or perhaps
there are paths like "www-frotz", "www.frotz", etc (i.e. "www" followed by
a byte whose ASCII value comes before '/') involved, and unpack_trees()
machinery the merge-recursive internally uses are getting confused about
D/F conflicts. To diagnose it, it would be helpful to get the full output
from these two commands:
git ls-tree -r -t HEAD (before cherry-pick)
git ls-tree -r -t 301a (the commit you are transplanting)
Thanks
---
diff --git a/t/t3506-cherry-pick-addremove.sh b/t/t3506-cherry-pick-addremove.sh
new file mode 100755
index 0000000..e7dcd77
--- /dev/null
+++ b/t/t3506-cherry-pick-addremove.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='unrelated files added by our side'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ test_commit A &&
+ git branch side &&
+ test_commit B &&
+ test_commit C &&
+
+ git checkout side &&
+ test_commit D &&
+ test_commit E &&
+
+ git checkout master
+'
+
+test_expect_success 'cherry-pick' '
+ git reset --hard C &&
+ git cherry-pick side &&
+ git grep E E.t
+'
+
+test_done
^ permalink raw reply related
* Re: git-cherry-pick problem - directory not touched by commit is marked "added by us"
From: Junio C Hamano @ 2010-01-07 3:35 UTC (permalink / raw)
To: Hakim Cassimally; +Cc: git, Sam Vilain
In-Reply-To: <7v4omyfv6h.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> What "cherry-pick" internally does is to run:
>
> git merge-recursive 301a^ -- HEAD 301a
>
> That is, "We are at HEAD; consolidate the change since 301a^ to 301a into
> our state, and leave the result in the index and the work tree". Then it
> commits the result. One thing to try is to see if this gives the same
> kind of breakage.
There actually is another possibility; we used to run inside "cherry-pick"
git merge-resolve 301a^ -- HEAD 301a
instead. The request is the same but it uses a different algorithm, so if
one fails and one succeeds, that might give us a better clue to figure out
what is going on.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2010, #02 draft; Wed, 06)
From: Junio C Hamano @ 2010-01-07 3:43 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git
In-Reply-To: <20100107122334.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Junio C Hamano <gitster@pobox.com>
>
>> I am experimenting with ideas to better manage the periodic "What's
>> cooking" messages, and here is one of such attempt based on the current
>> draft of the upcoming "2010 Jan, issue #02".
>> ...
>> Moved from [New Topics] to [Cooking]
>>
>> * da/difftool (2009-12-22) 2 commits
>> - - git-difftool: Add '--gui' for selecting a GUI tool
>> - - t7800-difftool: Set a bogus tool for use by tests
>> + (merged to 'next' on 2010-01-06 at e957395)
>> + + git-difftool: Add '--gui' for selecting a GUI tool
>> + + t7800-difftool: Set a bogus tool for use by tests
>>
>> * jh/gitweb-cached (2010-01-03) 4 commits
>> - gitweb: Makefile improvements
>> - gitweb: Optionally add "git" links in project list page
>> - gitweb: Add option to force version match
>> - gitweb: Load checking
>> +
>> +Will merge to 'next', unless I hear objections within a few days.
>
> For what it's worth, I think this new format very easy to spot the
> differences and much nicer.
This is _not_ a "new format" per-se, but is primarily a demonstration of
the external diff feature (all in 'todo' branch).
While I think it is easier to view when I am interested in the differences
since the previous issue (and that was why I wrote it) than the full list,
its "incremental" nature cuts both ways.
The primary reason I send out "What's cooking" is to make sure people are
aware of topics that are _not_ going forward, to encourage them to be
involved by helping such topics. When some topics are moved from Cooking
to Stalled category, they will show up in the incremental report, but
after that, until they start moving again, they won't be shown in the
incremental report. That defeats the whole point of sending the "What's
cooking" messages.
Topics that are in motion are watched actively by testers, developers and
documenters. They don't need as much exposure as stalled ones.
^ permalink raw reply
* [PATCH] Fix segfault in fast-export
From: Mike Mueller @ 2010-01-07 3:58 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]
Hi all,
I'm working on a C++ static analyzer (Vigilant Sentry), and git
is one of my test subjects. In git-1.6.6, I found a crash in the
fast-export command:
The problem is in builtin-fast-export.c, function export_marks:
f = fopen(file, "w");
if (!f)
error("Unable to open marks file %s for writing.", file);
for (i = 0; i < idnums.size; i++) {
if (deco->base && deco->base->type == 1) {
mark = ptr_to_mark(deco->decoration);
if (fprintf(f, ":%"PRIu32" %s\n", mark,
sha1_to_hex(deco->base->sha1)) < 0) {
e = 1;
break;
}
}
deco++;
}
e |= ferror(f);
e |= fclose(f);
If fopen() fails, the error message is printed, but the function
doesn't exit. The subsequent calls to fprintf and/or ferror will
fail because f is NULL. A simple way to reproduce is to export
to a path you don't have write access to:
$ git fast-export --export-marks=/foo
error: Unable to open marks file /foo for writing.
Segmentation fault (core dumped)
I've attached a trivial patch that calls die_errno instead of
error, so the program exits if f is NULL.
Regards,
Mike
--
Mike Mueller
mmueller@vigilantsw.com
http://www.vigilantsw.com/
[-- Attachment #2: git-fast-export.patch --]
[-- Type: text/x-diff, Size: 449 bytes --]
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index b0a4029..963e89b 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -503,7 +503,7 @@ static void export_marks(char *file)
f = fopen(file, "w");
if (!f)
- error("Unable to open marks file %s for writing.", file);
+ die_errno("Unable to open marks file %s for writing", file);
for (i = 0; i < idnums.size; i++) {
if (deco->base && deco->base->type == 1) {
^ permalink raw reply related
* Re: [PATCH 1/2] t7002: set test prerequisite "external-grep" if supported
From: Junio C Hamano @ 2010-01-07 4:29 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <7v4omyhc7h.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Add another test to set prerequisite "external-grep" if the current
>> build supports external grep. This can be used to skip external grep
>> only tests on builds that do not support this optimization.
>
> Thanks. We seem to spell our prerequistes in a single-word, all-caps, so
> I'll change this new one to EXTGREP in both [1/2] and [2/2].
Sorry, but I had this "Sheesh, why didn't I think of that earlier before
wasting Nguyễn's time" moment.
Why don't we just test what we _want to_ test? After all what a67e281
(grep: do not do external grep on skip-worktree entries, 2009-12-30)
wanted to make sure was this:
"git grep" (without --cached) should grep from the index for paths
that are marked as skip-worktree.
So how about writing some string that does not appear in the version in
the index in the work tree file, and run "git grep" to make sure it
doesn't find it?
Yes, some implementations/builds of "git grep" may not even try to cheat
and run external grep and for them the test _should_ succeed (but your
logic to check with ce_skip_worktree() in grep_cache() may be broken by
later patch while you are looking the other way), and some will try to
cheat and the fix was about not letting them.
So by writing the test to check the desired outcome, instead of writing it
for the particular implementation of using external grep optimization, you
will catch both kinds of breakages.
Perhaps something like this (untested, of course)?
test_expect_success 'strings in work tree files are not found for skip-wt paths' '
no="no such string in the index" &&
test_must_fail git grep -e "$no" --cached file &&
git update-index --skip-worktree file &&
echo "$no" >file &&
test_must_fail git grep -e "$no" file &&
git update-index --no-skip-worktree file &&
git grep -e "$no" file
'
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2010, #02 draft; Wed, 06)
From: Sverre Rabbelier @ 2010-01-07 4:42 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano
In-Reply-To: <7vaawqna55.fsf@alter.siamese.dyndns.org>
Heya,
On Wed, Jan 6, 2010 at 17:25, Junio C Hamano <gitster@pobox.com> wrote:
> * sr/gfi-options (2009-12-04) 7 commits
> - fast-import: add (non-)relative-marks feature
> - fast-import: allow for multiple --import-marks= arguments
> - fast-import: test the new option command
> - fast-import: add option command
> - fast-import: add feature command
> - fast-import: put marks reading in its own function
> - fast-import: put option parsing code in separate functions
>
> http://thread.gmane.org/gmane.comp.version-control.git/134540
>
> I haven't seen comments on this round, and I am tempted to merge it to
> 'next' soonish. Please file complaints before I do so if people have
> objections.
Shawn, it would be awesome if you could review this and let me know
what you think of it.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: checkout -m dumping core
From: Junio C Hamano @ 2010-01-07 5:07 UTC (permalink / raw)
To: Daniel; +Cc: Git List, Tomas Carnecky
In-Reply-To: <2b9e2ea1.461a9565.4b4445be.38bda@o2.pl>
Daniel <mjucde@o2.pl> writes:
> Does this patch help?
There actually is no point running parse_commit() at that point, as the
code obtained old->commit from lookup_commit_reference_gently() in
switch_branches() which must have already parsed it. We do use it to try
switching the branches before falling back to the --merge mode (or we
notice old->commit does not exist and switch from an empty tree).
Instead of silently dying without diagnosing what paths are problematic
like your patch does, we can unsquelch the error from the unpack_tree() we
run before falling back to the --merge mode, and allow it to notice and
report the paths that will be overwritten (since you do not have HEAD,
everything in the work tree will be overwritten). Perhaps like this.
builtin-checkout.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..2708669 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -397,7 +397,7 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.initial_checkout = is_cache_unborn();
topts.update = 1;
topts.merge = 1;
- topts.gently = opts->merge;
+ topts.gently = opts->merge && old->commit;
topts.verbose_update = !opts->quiet;
topts.fn = twoway_merge;
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -422,7 +422,13 @@ static int merge_working_tree(struct checkout_opts *opts,
struct merge_options o;
if (!opts->merge)
return 1;
- parse_commit(old->commit);
+
+ /*
+ * Without old->commit, the below is the same as
+ * the two-tree unpack we already tried and failed.
+ */
+ if (!old->commit)
+ return 1;
/* Do more real merge */
^ permalink raw reply related
* Re: [spf:guess] Re: [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Eric Wong @ 2010-01-07 6:49 UTC (permalink / raw)
To: Sam Vilain; +Cc: Andrew Myrick, git
In-Reply-To: <4B4510EE.4090504@vilain.net>
Sam Vilain <sam@vilain.net> wrote:
> Eric Wong wrote:
> > Andrew Myrick <amyrick@apple.com> wrote:
> >
> >> diff --git a/git-svn.perl b/git-svn.perl
> >
> > Hi Andrew,
> >
> > I'll again defer to Sam for Acks on these. Test cases would be nice to
> > have, too.
> >
>
> They look fine to me, agreed a test case would be nice and make sure the
> features aren't lost later inadvertently.
Thanks Sam,
Shall I consider this an Ack and push Andrew's new changes
up while waiting for test cases?
--
Eric Wong
^ permalink raw reply
* [PATCH v2 0/5] Lazily generate header dependencies
From: Jonathan Nieder @ 2010-01-07 7:13 UTC (permalink / raw)
To: Nanako Shiraishi
Cc: Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100101090550.6117@nanako3.lavabit.com>
Nanako Shiraishi wrote:
> Junio, could you tell us what happened to this thread?
>
> Makefile improvements. No discussion.
My bad. The previous version was very rough because I was not sure
yet how this could help in making the header dependency rules more
maintainable. If all compilers worth using support something like
gcc's -MD option (does MSVC?), we could switch over completely;
otherwise, we need some way to use the generated dependencies to
check the static ones, or the static ones will go stale.
That is, maybe something like this. With these patches applied,
running
echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
make clean
make
make CHECK_HEADER_DEPENDENCIES=YesPlease
will fail unless the dependency rules in the Makefile account for
all #includes gcc noticed with the current configuration.
Patch 5 is a little rough around the edges, but I am hoping it
will convey the idea.
Enjoy,
Jonathan Nieder (5):
Makefile: rearrange dependency rules
Makefile: clear list of default rules
Makefile: list generated object files in OBJECTS macro
Makefile: lazily compute header dependencies
Teach Makefile to check header dependencies
.gitignore | 1 +
Makefile | 158 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 130 insertions(+), 29 deletions(-)
^ permalink raw reply
* [PATCH 1/5] Makefile: rearrange dependency rules
From: Jonathan Nieder @ 2010-01-07 7:16 UTC (permalink / raw)
To: Git Mailing List
Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>
Collect header dependency rules after the pattern rules to make
it easier to modify them all at once. No change in behavior is
intended.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/Makefile b/Makefile
index 8ce6fd7..fa08535 100644
--- a/Makefile
+++ b/Makefile
@@ -1630,6 +1630,11 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.o \
+ $(patsubst git-%$X,%.o,$(PROGRAMS))
+XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
+ xdiff/xmerge.o xdiff/xpatience.o
+
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
@@ -1637,6 +1642,14 @@ git.o git.spec \
%.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(GIT_OBJS): $(LIB_H)
+http.o http-walker.o http-push.o: http.h
+$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
+builtin-revert.o wt-status.o: wt-status.h
+
+$(XDIFF_OBJS): xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
+ xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+
exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1661,10 +1674,6 @@ git-imap-send$X: imap-send.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
-http.o http-walker.o http-push.o: http.h
-
-http.o http-walker.o: $(LIB_H)
-
git-http-fetch$X: revision.o http.o http-walker.o http-fetch.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL)
@@ -1676,18 +1685,9 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.h
-
$(LIB_FILE): $(LIB_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
-XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
- xdiff/xmerge.o xdiff/xpatience.o
-$(XDIFF_OBJS): xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
- xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
-
$(XDIFF_LIB): $(XDIFF_OBJS)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)
--
1.6.6.rc2
^ permalink raw reply related
* [PATCH 2/5] Makefile: clear list of default rules
From: Jonathan Nieder @ 2010-01-07 7:18 UTC (permalink / raw)
To: Git Mailing List
Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>
The git makefile never uses any default implicit rules. If a
prerequisite for one of the intended rules is missing, a default
rule can be used in its place:
$ make var.s
CC var.s
$ rm var.c
$ make var.o
as -o var.o var.s
Avoiding the default rules increases performance and avoids
hard-to-debug behaviour. Especially, once the scope of the
%.o: %.c pattern rule is restricted, we should not fall back
to the default %.o: %.c pattern rule.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
'make -d' reveals that GNU make still ponders the default rules with
this patch applied, though at least it does not use them any more. Is
it possible to set something like the make '-r' option from within a
makefile?
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index fa08535..9a5d897 100644
--- a/Makefile
+++ b/Makefile
@@ -1635,6 +1635,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.o \
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
+.SUFFIXES:
+
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
--
1.6.6.rc2
^ permalink raw reply related
* [PATCH 3/5] Makefile: add OBJECTS variable listing object files
From: Jonathan Nieder @ 2010-01-07 7:19 UTC (permalink / raw)
To: Git Mailing List
Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>
To find the generated dependencies to include, we will need a
comprehensive list of all object file targets. To make sure it
is truly comprehensive, restrict the scope of the
%.o pattern rule to only generate objects in that list.
Attempts to build other object files will fail loudly:
$ touch foo.c
$ make foo.o
make: *** No rule to make target `foo.o'. Stop.
providing a reminder to add the new object to the OBJECTS list.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 34 ++++++++++++++++++++--------------
1 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile
index 9a5d897..87de3c3 100644
--- a/Makefile
+++ b/Makefile
@@ -388,6 +388,18 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
# Empty...
EXTRA_PROGRAMS =
+TEST_PROGRAMS += test-chmtime$X
+TEST_PROGRAMS += test-ctype$X
+TEST_PROGRAMS += test-date$X
+TEST_PROGRAMS += test-delta$X
+TEST_PROGRAMS += test-dump-cache-tree$X
+TEST_PROGRAMS += test-genrandom$X
+TEST_PROGRAMS += test-match-trees$X
+TEST_PROGRAMS += test-parse-options$X
+TEST_PROGRAMS += test-path-utils$X
+TEST_PROGRAMS += test-sha1$X
+TEST_PROGRAMS += test-sigchain$X
+
# ... and all the rest that could be moved out of bindir to gitexecdir
PROGRAMS += $(EXTRA_PROGRAMS)
PROGRAMS += git-fast-import$X
@@ -1634,14 +1646,20 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.o \
$(patsubst git-%$X,%.o,$(PROGRAMS))
XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
xdiff/xmerge.o xdiff/xpatience.o
+TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(TEST_OBJS)
+
+ASM_SRC := $(wildcard $(OBJECTS:o=S))
+ASM_OBJ := $(ASM_SRC:S=o)
+C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
.SUFFIXES:
-%.o: %.c GIT-CFLAGS
+$(C_OBJ): %.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S GIT-CFLAGS
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
$(GIT_OBJS): $(LIB_H)
@@ -1757,18 +1775,6 @@ endif
### Testing rules
-TEST_PROGRAMS += test-chmtime$X
-TEST_PROGRAMS += test-ctype$X
-TEST_PROGRAMS += test-date$X
-TEST_PROGRAMS += test-delta$X
-TEST_PROGRAMS += test-dump-cache-tree$X
-TEST_PROGRAMS += test-genrandom$X
-TEST_PROGRAMS += test-match-trees$X
-TEST_PROGRAMS += test-parse-options$X
-TEST_PROGRAMS += test-path-utils$X
-TEST_PROGRAMS += test-sha1$X
-TEST_PROGRAMS += test-sigchain$X
-
all:: $(TEST_PROGRAMS)
# GNU make supports exporting all variables by "export" without parameters.
--
1.6.6.rc2
^ permalink raw reply related
* [PATCH 4/5] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2010-01-07 7:23 UTC (permalink / raw)
To: Git Mailing List
Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>
Use the gcc -MMD -MP -MF options to generate dependency rules as
a byproduct when building .o files.
This feature has to be optional (MSVC does not seem to support
anything like this), so unfortunately it does not make the
Makefile much easier to maintain. The feature is enabled by the
COMPUTE_HEADER_DEPENDENCIES variable, which is unset by default.
The generated Makefile fragments are saved in deps/
subdirectories of each directory containing object files. These
directories are generated if missing at the start of each build.
A dependency on $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
avoids needlessly regenerating files when the directories'
timestamps change.
gcc learned the -MMD -MP -MF options in version 3.0, so most gcc
users should have them by now.
The dependencies this option computes are more specific than the
rough estimate hard-coded in the Makefile, greatly speeding up
rebuilds when only a little-used header file has changed.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Timings:
for arg in YesPlease ""
do
{
echo NO_CURL=1
echo NO_TCLTK=1
echo NO_PERL=1
echo COMPUTE_HEADER_DEPENDENCIES="$arg"
} >config.mak
make
make clean
time -p make
touch levenshtein.h
time -p make
make clean
done >/dev/null
Build COMPUTE_HEADER_DEPENDENCIES real user sys
first YesPlease 55.06 45.13 5.23
second YesPlease 3.13 2.04 0.79
first 55.45 45.49 4.99
second 53.14 43.19 4.70
.gitignore | 1 +
Makefile | 40 ++++++++++++++++++++++++++++++++++++----
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore
index ac02a58..803247f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
*.exe
*.[aos]
*.py[co]
+*.o.d
*+
/config.mak
/autom4te.cache
diff --git a/Makefile b/Makefile
index 87de3c3..578843c 100644
--- a/Makefile
+++ b/Makefile
@@ -221,6 +221,9 @@ all::
# DEFAULT_EDITOR='~/bin/vi',
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
+# when an unrelated header file changes and your compiler supports it.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1653,15 +1656,38 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
ASM_OBJ := $(ASM_SRC:S=o)
C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
+dep_dir_dep := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+
+$(dep_dirs):
+ mkdir -p $@
+else
+dep_dirs =
+dep_dir_dep =
+endif
+
.SUFFIXES:
-$(C_OBJ): %.o: %.c GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS FORCE
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(dep_dir_dep)
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+# Take advantage of gcc's dependency generation
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
$(GIT_OBJS): $(LIB_H)
http.o http-walker.o http-push.o: http.h
$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
@@ -1670,6 +1696,9 @@ builtin-revert.o wt-status.o: wt-status.h
$(XDIFF_OBJS): xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+dep_args =
+endif
+
exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1794,7 +1823,9 @@ test-delta$X: diff-delta.o patch-delta.o
test-parse-options$X: parse-options.o
+ifndef COMPUTE_HEADER_DEPENDENCIES
test-parse-options.o: parse-options.h
+endif
.PRECIOUS: $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
@@ -1954,6 +1985,7 @@ clean:
$(LIB_FILE) $(XDIFF_LIB)
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
+ $(RM) -r $(dep_dirs)
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
$(RM) -r autom4te.cache
$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
--
1.6.6.rc2
^ permalink raw reply related
* [PATCH/RFC 5/5] Teach Makefile to check header dependencies
From: Jonathan Nieder @ 2010-01-07 7:30 UTC (permalink / raw)
To: Git Mailing List
Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>
Portability means we cannot completely switch over to
automatically generated dependencies on header files, since some
compilers do not support them. This would seem to lead to a
dangerous situation in which the hand-written dependency rules
are needed for some configurations but poorly maintained because
most configurations do not use them.
Luckily, there is a way out: as part of testing git, ask the
build system to verify that the hand-written dependency rules are
consistent with the automatically generated ones. This patch is
a start towards that goal.
The actual patch requires a few steps:
1. Separate out a USE_COMPUTED_HEADER_DEPENDENCIES option to
tell make to use the makefile snippets stored in deps/*
without necessarily regenerating them;
2. Add a PRINT_HEADER_DEPENDENCIES option to turn on
USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
print its prerequisites instead of compiling anything;
3. Add a CHECK_HEADER_DEPENDENCIES option to turn off
USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
check that its prerequisites includes all files listed by
'make -s PRINT_HEADER_DEPENDENCIES=YesPlease $@' instead of
compiling anything.
With this patch applied,
echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
make clean
make
make CHECK_HEADER_DEPENDENCIES=YesPlease
produces a useful error message:
CHECK fast-import.o
missing dependencies: exec_cmd.h
make: *** [fast-import.o] Error 1
Probably we should check for missing deps/%.o.d files to avoid
false negatives if some are missing.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I look forward to your thoughts.
This is a bit clunky, but it is useful enough to detect a few problems
with the current dependency rules. Patches should follow soon.
Makefile | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 578843c..e642a24 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,8 @@ all::
#
# Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
# when an unrelated header file changes and your compiler supports it.
+#
+# Define CHECK_HEADER_DEPENDENCIES after a successful build to find problems.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1064,6 +1066,28 @@ endif
-include config.mak.autogen
-include config.mak
+ifdef PRINT_HEADER_DEPENDENCIES
+CHECK_HEADER_DEPENDENCIES = YesPlease
+endif
+
+ifdef CHECK_HEADER_DEPENDENCIES
+ifndef COMPUTE_HEADER_DEPENDENCIES
+$(error checking dependencies requires build with COMPUTE_HEADER_DEPENDENCIES)
+endif
+endif
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+ifdef CHECK_HEADER_DEPENDENCIES
+ifdef PRINT_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+else
+USE_COMPUTED_HEADER_DEPENDENCIES =
+endif
+else
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+endif
+endif
+
ifdef SANE_TOOL_PATH
SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
@@ -1669,14 +1693,48 @@ endif
.SUFFIXES:
+ifdef CHECK_HEADER_DEPENDENCIES
+
+ifdef PRINT_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c FORCE
+ echo $^
+$(ASM_OBJ): %.o: %.S FORCE
+ echo $^
+else
+missing_deps = $(filter-out $^, \
+ $(shell $(MAKE) -s PRINT_HEADER_DEPENDENCIES=1 $@))
+
+$(C_OBJ): %.o: %.c FORCE
+ @set -e; echo CHECK $@; \
+ missing_deps="$(missing_deps)"; \
+ if test "$$missing_deps"; \
+ then \
+ echo missing dependencies: $$missing_deps; \
+ false; \
+ fi
+$(ASM_OBJ): %.o: %.S FORCE
+ @set -e; echo CHECK $@; \
+ missing_deps="$(missing_deps)"; \
+ if test "$$missing_deps"; \
+ then \
+ echo missing dependencies: $$missing_deps; \
+ false; \
+ fi
+endif
+
+else
+
$(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS FORCE
- $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(dep_dir_dep)
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
-ifdef COMPUTE_HEADER_DEPENDENCIES
+endif
+
+%.s: %.c GIT-CFLAGS FORCE
+ $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
# Take advantage of gcc's dependency generation
# See <http://gcc.gnu.org/gcc-3.0/features.html>.
dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
@@ -1684,9 +1742,6 @@ dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
ifneq ($(dep_files),)
include $(dep_files)
endif
-
-dep_file = $(dir $@)deps/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
else
$(GIT_OBJS): $(LIB_H)
http.o http-walker.o http-push.o: http.h
@@ -1695,7 +1750,12 @@ builtin-revert.o wt-status.o: wt-status.h
$(XDIFF_OBJS): xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+endif
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
dep_args =
endif
@@ -1823,7 +1883,7 @@ test-delta$X: diff-delta.o patch-delta.o
test-parse-options$X: parse-options.o
-ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef USE_COMPUTED_HEADER_DEPENDENCIES
test-parse-options.o: parse-options.h
endif
--
1.6.6.rc2
^ permalink raw reply related
* Re: [PATCH 2/5] Makefile: use target-specific variable to pass flags to cc
From: Jonathan Nieder @ 2010-01-07 7:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20100106080504.GC7298@progeny.tock>
Jonathan Nieder wrote:
> diff --git a/Makefile b/Makefile
> index ba4d071..81190a6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1467,20 +1467,19 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
> strip: $(PROGRAMS) git$X
> $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
>
> -git.o: git.c common-cmds.h GIT-CFLAGS
> - $(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
> - '-DGIT_HTML_PATH="$(htmldir_SQ)"' \
> - $(ALL_CFLAGS) -o $@ -c $(filter %.c,$^)
> +git.o: common-cmds.h
> +git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
> + '-DGIT_HTML_PATH="$(htmldir_SQ)"'
>
[...]
One annoying feature I wasn't thinking of: the values of
target-specific variables propagate to the dependencies of a target
(why? I can't imagine), and GIT-CFLAGS keeps on changing because of
this.
Maybe a new CMD_CFLAGS variable is needed for this, i.e. something
like the following squashed in.
diff --git a/Makefile b/Makefile
index 2580e23..d20e456 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,7 +1468,7 @@ strip: $(PROGRAMS) git$X
$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
git.o: common-cmds.h
-git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.o: CMD_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
'-DGIT_HTML_PATH="$(htmldir_SQ)"'
git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
@@ -1476,7 +1476,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
builtin-help.o: common-cmds.h
-builtin-help.o: ALL_CFLAGS += \
+builtin-help.o: CMD_CFLAGS += \
'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
'-DGIT_MAN_PATH="$(mandir_SQ)"' \
'-DGIT_INFO_PATH="$(infodir_SQ)"'
@@ -1630,28 +1630,31 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+# This can vary by target
+CMD_CFLAGS = $(ALL_CFLAGS)
+
%.o: %.c GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+ $(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
%.s: %.c GIT-CFLAGS .FORCE-LISTING
- $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+ $(QUIET_CC)$(CC) -S $(CMD_CFLAGS) $<
%.o: %.S GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+ $(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
-exec_cmd.o: ALL_CFLAGS += \
+exec_cmd.o: CMD_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
'-DBINDIR="$(bindir_relative_SQ)"' \
'-DPREFIX="$(prefix_SQ)"'
-builtin-init-db.o: ALL_CFLAGS += \
+builtin-init-db.o: CMD_CFLAGS += \
-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
-config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+config.o: CMD_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
-http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
+http.o: CMD_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
ifdef NO_EXPAT
http-walker.o: http.h
-http-walker.o: ALL_CFLAGS += -DNO_EXPAT
+http-walker.o: CMD_CFLAGS += -DNO_EXPAT
endif
git-%$X: %.o $(GITLIBS)
^ permalink raw reply related
* Re: [spf:guess] Re: [spf:guess] Re: [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Sam Vilain @ 2010-01-07 9:05 UTC (permalink / raw)
To: Eric Wong; +Cc: Andrew Myrick, git
In-Reply-To: <20100107064916.GA8557@dcvr.yhbt.net>
On Wed, 2010-01-06 at 22:49 -0800, Eric Wong wrote:
> Sam Vilain <sam@vilain.net> wrote:
> > Eric Wong wrote:
> > > Andrew Myrick <amyrick@apple.com> wrote:
> > >
> > >> diff --git a/git-svn.perl b/git-svn.perl
> > >
> > > Hi Andrew,
> > >
> > > I'll again defer to Sam for Acks on these. Test cases would be nice to
> > > have, too.
> > >
> >
> > They look fine to me, agreed a test case would be nice and make sure the
> > features aren't lost later inadvertently.
>
> Thanks Sam,
>
> Shall I consider this an Ack and push Andrew's new changes
> up while waiting for test cases?
Sure, the intent looks fine and given that Andrew has likely been using
it a lot it's probably right. So yes, that's an ack. Hopefully I'll
find time to push along some tests soon.
Cheers,
Sam
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox