* Re: [PATCH] git-am: don't ignore --keep (-k) option
From: Junio C Hamano @ 2009-11-27 20:03 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list
In-Reply-To: <87638zm38r.fsf_-_@meyering.net>
Jim Meyering <jim@meyering.net> writes:
> I started looking at git-am.sh and spotted what appears to be a typo.
> There is only that one use of $keep_subject, so its value currently
> comes from the environment.
>
> From 02f7e6433b5db8b18a4cccf58c302159c2f54fa5 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Wed, 25 Nov 2009 09:10:46 +0100
> Subject: [PATCH] git-am: don't ignore --keep (-k) option
>
> Fix typo in variable name: s/keep_subject/keep/.
>
> Signed-off-by: Jim Meyering <meyering@redhat.com>
At the level of "what does each line of the code do", this is a fix, but
as we do a lot more than just stripping "[PATCH] " from the beginning of
the Subject: line these days, I think we are better off declaring defeat
in this particular codepath and not doing anything here.
Adding "[PATCH] " is no longer "keeping the original subject" anyway. It
is "without knowing what we already stripped, adding one random string
that could have been what we removed".
I also have to wonder why $dotest/info does not have the [PATCH] or
whatever prefix that we were told not to strip in this codepath. After
all, we are running "git mailinfo" with $keep option to produce that file,
so if that part is working correctly, we shouldn't even have to have this
"add [PATCH] back" trick to begin with.
What am I missing???
> ---
> git-am.sh | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 151512a..f353e73 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -578,7 +578,7 @@ do
> sed -e '1,/^$/d' >"$dotest/msg-clean"
> else
> SUBJECT="$(sed -n '/^Subject/ s/Subject: //p' "$dotest/info")"
> - case "$keep_subject" in -k) SUBJECT="[PATCH] $SUBJECT" ;; esac
> + case "$keep" in -k) SUBJECT="[PATCH] $SUBJECT" ;; esac
>
> (printf '%s\n\n' "$SUBJECT"; cat "$dotest/msg") |
> git stripspace > "$dotest/msg-clean"
> --
> 1.6.6.rc0.236.ge0b94
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Johannes Schindelin @ 2009-11-27 20:07 UTC (permalink / raw)
To: Jeff King; +Cc: James Pickens, Junio C Hamano, git
In-Reply-To: <20091127180235.GA26633@coredump.intra.peff.net>
Hi,
On Fri, 27 Nov 2009, Jeff King wrote:
> On Fri, Nov 27, 2009 at 11:53:42AM +0100, Johannes Schindelin wrote:
>
> > > If only somebody had written a "pager.status" configuration variable,
> > > you could use that. Oh wait. I did. And it shipped in v1.6.0.
> >
> > And it makes things inconsistent. That is why I do not use it.
>
> Then you can not use this configuration variable, too. Has the existence
> of pager.status, since you do not use it, been a problem for you so far?
No, since none of the people I helped use it.
> > Do you work on 10 different computers? I do. And nothing is more
> > unnerving than the same command producing something different on the
> > different computers.
>
> Yes, as a matter of fact, I do work on 10 different computers. I'm sorry
> that you find managing your configuration so challenging. But if you
> don't use the configuration variable, then your own personal setup is
> totally irrelevant.
As I just demonstrated, this is a false statement.
> If your argument is that this lack of consistency will irritate users,
> you need to show that:
>
> 1. There are users who switch between a large number of setups, but
> will not apply config consistently.
This is a strawman, and you should be ashamed to put it here. Just
because nobody does what you actively encourage does not mean that the
encouraged procedure is good, or for that matter, helps anybody but you.
Just think about it. If you plan to change the side cars are supposed to
drive on, it is not enough to have a nice cozy committee deciding on it in
some little room somewhere in Wyoming. Especially not if they decide that
you can drive on the other side if you put a sticker "I am a right-wing
driver" on your car.
It is inconsistent, and it is violating the law of the least surprise.
> And the GitTogether had a "users complain about git, and we try to
> listen" session.
Oh, that makes me so happy. <sarcasm>Soooo happy</sarcasm>. So it was an
ivory tower meeting, once again?
Ciao,
Dscho
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 07/11] run-command: support input-fd
From: Johannes Sixt @ 2009-11-27 20:14 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <40aa078e0911270639n1de36517w5fdf6ef38e931b19@mail.gmail.com>
On Freitag, 27. November 2009, Erik Faye-Lund wrote:
> On Thu, Nov 26, 2009 at 10:53 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> > On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> >> @@ -327,7 +327,10 @@ int start_async(struct async *async)
> >> {
> >> int pipe_out[2];
> >>
> >> - if (pipe(pipe_out) < 0)
> >> + if (async->out) {
> >> + pipe_out[0] = dup(async->out);
> >> + pipe_out[1] = dup(async->out);
> >> + } else if (pipe(pipe_out) < 0)
> >> return error("cannot create pipe: %s", strerror(errno));
> >> async->out = pipe_out[0];
> >
> > Hm. If async->out != 0:
> >
> > pipe_out[0] = dup(async->out);
> > async->out = pipe_out[0];
> >
> > This is confusing.
>
> What do you find confusing about it? The idea is to use a provided
> bi-directional fd instead of a pipe if async->out is non-zero. The
> currently defined rules for async is that async->out must be zero
> (since the structure should be zero-initialized).
It is just the code structure that is confusing. It should be
if (async->out) {
/* fd was provided */
do all that is needed in this case
} else {
/* fd was requested */
do all for this other case
}
/* nothing to do anymore here */
(Of course, this should only replace the part that is cited above, not the
whole function.)
> > Moreover, you are assigning (a dup of) the same fd to the writable end.
> > This assumes a bi-directional channel. I don't yet know what I should
> > think about this (haven't studied the later patches, yet).
>
> Indeed it does. Do we want to extend it to support a set of
> unidirectional channels instead?
Yes, I think so. We could pass a regular int fd[2] array around with the clear
definition that both can be closed independently, i.e. one must be a dup() of
the other. struct async would also have such an array.
Speaking of dup(): The underlying function is DuplicateHandle(), and its
documentation says:
"You should not use DuplicateHandle to duplicate handles to the following
objects: ... o Sockets. ... use WSADuplicateSocket."
But then the docs of WSADuplicateSocket() talk only about duplicating a socket
to a separate process. Perhaps DuplicateHandle() of a socket within the same
process Just Works?
-- Hannes
^ permalink raw reply
* Re: [PATCH] git-am: don't ignore --keep (-k) option
From: Jim Meyering @ 2009-11-27 20:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7vy6lrka69.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>> I started looking at git-am.sh and spotted what appears to be a typo.
>> There is only that one use of $keep_subject, so its value currently
>> comes from the environment.
>>
>> From 02f7e6433b5db8b18a4cccf58c302159c2f54fa5 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering@redhat.com>
>> Date: Wed, 25 Nov 2009 09:10:46 +0100
>> Subject: [PATCH] git-am: don't ignore --keep (-k) option
>>
>> Fix typo in variable name: s/keep_subject/keep/.
>>
>> Signed-off-by: Jim Meyering <meyering@redhat.com>
>
> At the level of "what does each line of the code do", this is a fix, but
> as we do a lot more than just stripping "[PATCH] " from the beginning of
> the Subject: line these days, I think we are better off declaring defeat
> in this particular codepath and not doing anything here.
Sounds fine to me.
Glad you're keeping everything in perspective.
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 08/11] daemon: use explicit file descriptor
From: Johannes Sixt @ 2009-11-27 20:23 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <40aa078e0911270746x55946f52qd76dc4f9443aebc6@mail.gmail.com>
On Freitag, 27. November 2009, Erik Faye-Lund wrote:
> On Fri, Nov 27, 2009 at 3:23 PM, Erik Faye-Lund
> <kusmabite@googlemail.com> wrote:
> > At the very least, I should remove the
> > "dup2(incoming, 1)"-call, but I'm open to other suggestions. Perhaps I
> > can change this patch to do the entire socket-passing (which is
> > currently in the next patch)?
No, an infrastructure change in a separate patch is good.
> Something along these lines?
>
> ---8<---
> - cld.in = cld.out = fd;
> + cld.in = dup(fd);
> + cld.out = fd;
>...
> - dup2(incoming, 0);
> - dup2(incoming, 1);
> - close(incoming);
> -
> - exit(execute(0, addr));
> + exit(execute(incoming, addr));
> ---8<---
Yes, this looks very good.
> When I think more about it, I might've broken the inetd-mode as it
> should communicate over stdin and stdout (not just stdin as it would
> try to do now)... I don't know the inetd internals, but this frightens
> me a bit.
Do we need inetd mode on Windows? At one time a looked for a inetd-like
service, but couldn't find one.
-- Hannes
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 08/11] daemon: use explicit file descriptor
From: Johannes Sixt @ 2009-11-27 20:28 UTC (permalink / raw)
To: msysgit; +Cc: kusmabite, git, dotzenlabs
In-Reply-To: <200911272123.45163.j6t@kdbg.org>
On Freitag, 27. November 2009, Johannes Sixt wrote:
> On Freitag, 27. November 2009, Erik Faye-Lund wrote:
> > When I think more about it, I might've broken the inetd-mode as it
> > should communicate over stdin and stdout (not just stdin as it would
> > try to do now)... I don't know the inetd internals, but this frightens
> > me a bit.
>
> Do we need inetd mode on Windows? At one time a looked for a inetd-like
> service, but couldn't find one.
How foolish of me. This affects all platforms. Of course it is an important to
keep inetd mode.
-- Hannes
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Jeff King @ 2009-11-27 20:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, James Pickens, git
In-Reply-To: <7vpr73n7ns.fsf@alter.siamese.dyndns.org>
On Fri, Nov 27, 2009 at 10:29:11AM -0800, Junio C Hamano wrote:
> > If only somebody had written a "pager.status" configuration variable,
> > you could use that. Oh wait. I did. And it shipped in v1.6.0.
>
> Nice try but, "grep" and "status" are apples and oranges comparision.
Yes, I think you are right that the existence of pager.* does not
necessarily imply that there should be a config option for grep. But
that makes his example even more irrelevant: he is advocating that I use
a solution in this instance because he uses it in another instance, when
that solution is not even necessary in the other instance (and as I have
hopefully already made clear, is in my opinion inferior).
It is probably better to stay on the topic of the grep option, though.
-Peff
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Jeff King @ 2009-11-27 20:53 UTC (permalink / raw)
To: Uri Okrent; +Cc: Junio C Hamano, Johannes Schindelin, James Pickens, git
In-Reply-To: <4B101ED1.9000607@gmail.com>
On Fri, Nov 27, 2009 at 10:47:45AM -0800, Uri Okrent wrote:
> >Changing "grep" is too late for 1.7.0, but we are trying to find an easy
> >migration path like you mentioned in your message and that is exactly what
> >this thread is about.
>
> I wasn't actually suggesting we change grep for 1.7. As a matter of
> fact, my personal opinion (which I probably neglected to mention) is
> that grep default behavior should stay the same since it is semantically
> closer to unix (or gnu) grep.
Keeping consistency with non-git grep has been mentioned a few times in
this thread. I really don't understand how default file selection is
supposed to maintain consistency with non-git grep. Regular grep
defaults to stdin if no paths are given. That mode doesn't make any
sense for git grep.
So of the two options (grepping the list of files from the full tree, or
the list of files rooted at the current directory), how is one closer to
non-git grep than the other?
-Peff
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 09/11] daemon: use run-command api for async serving
From: Johannes Sixt @ 2009-11-27 20:59 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-10-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> static void check_dead_children(void)
> {
> - int status;
> - pid_t pid;
> -
> - while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
> - const char *dead = "";
> - remove_child(pid);
> - if (!WIFEXITED(status) || (WEXITSTATUS(status) > 0))
> - dead = " (with error)";
> - loginfo("[%"PRIuMAX"] Disconnected%s", (uintmax_t)pid, dead);
> - }
> + struct child **cradle, *blanket;
> + for (cradle = &firstborn; (blanket = *cradle);)
> + if (!is_async_alive(&blanket->async)) {
This would be the right place to call finish_async(). But since we cannot
wait, you invented is_async_alive(). But actually we are not only interested
in whether the process is alive, but also whether it completed successfully
so that we can add "(with error)". Would it make sense to have a function
finish_async_nowait() instead of is_async_alive() that (1) stresses the
start/finish symmetry and (2) can return more than just Boolean?
> + *cradle = blanket->next;
> + loginfo("Disconnected\n");
Here you are losing information about the pid, which is important to have in
the syslog. The \n should be dropped.
> + async.proc = async_execute;
> + async.data = ss;
> + async.out = incoming;
>
> - dup2(incoming, 0);
> - dup2(incoming, 1);
> + if (start_async(&async))
> + logerror("unable to fork");
> + else
> + add_child(&async, addr, addrlen);
> close(incoming);
> -
> - exit(execute(0, addr));
In start_command(), the convention is that fds that are provided by the caller
are closed by start_command() (even if there are errors). The close(incoming)
that you leave here indicates that you are not using the same convention with
start_async(). It would be nice to switch to the same convention.
-- Hannes
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Jeff King @ 2009-11-27 21:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: James Pickens, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0911272102430.4521@intel-tinevez-2-302>
On Fri, Nov 27, 2009 at 09:07:51PM +0100, Johannes Schindelin wrote:
> > Yes, as a matter of fact, I do work on 10 different computers. I'm sorry
> > that you find managing your configuration so challenging. But if you
> > don't use the configuration variable, then your own personal setup is
> > totally irrelevant.
>
> As I just demonstrated, this is a false statement.
I must have missed where you demonstrated it.
> > If your argument is that this lack of consistency will irritate users,
> > you need to show that:
> >
> > 1. There are users who switch between a large number of setups, but
> > will not apply config consistently.
>
> This is a strawman, and you should be ashamed to put it here. Just
How is this a strawman? A strawman would be me overstating an
exaggerated position by you and then arguing against it. All I have
claimed is that it is not sufficient for _you_ to be personally annoyed
by this existence of this option. You need to argue that there is a
significant group of people in the same situation who will be ignored.
Or have I mis-spoken in summarizing your claim that a "lack of
consistency will irritate users". Is that not your point?
> Just think about it. If you plan to change the side cars are supposed to
> drive on, it is not enough to have a nice cozy committee deciding on it in
> some little room somewhere in Wyoming. Especially not if they decide that
> you can drive on the other side if you put a sticker "I am a right-wing
> driver" on your car.
When the number of "git grep" crash fatalities rises above zero, maybe
this line of reasoning will be relevant.
I am talking about making software configurable so that people, in their
own private setups, can make the software work as they see fit. Yes, it
is possible for that setup to be visible to other people in some
situations. But I am arguing that we need to weigh the (in my opinion
substantial) inconvenience to users in their everyday work compared to
the inconvenience of one user sitting at another user's terminal (or
cutting and pasting commands, or running a script).
> > And the GitTogether had a "users complain about git, and we try to
> > listen" session.
>
> Oh, that makes me so happy. <sarcasm>Soooo happy</sarcasm>. So it was an
> ivory tower meeting, once again?
I don't know what to say. You complain and complain about how git is not
being responsive to users. Shawn organizes a session where people at
Google who are using git every day can try to make their complaints in
an organized forum where a bunch of developers will listen and talk
about ways we can address those complaints. And now you are mad about
that?
If you think we need a git conference where lots of users show up, I
think that's a great idea. But until you provide some suggestions about
how to organize such a thing, I don't see how you are helping anything.
-Peff
^ permalink raw reply
* Re: [PATCH] git-am: don't ignore --keep (-k) option
From: Junio C Hamano @ 2009-11-27 21:11 UTC (permalink / raw)
To: Jim Meyering; +Cc: Junio C Hamano, git list
In-Reply-To: <87y6lr203s.fsf@meyering.net>
Jim Meyering <jim@meyering.net> writes:
>> At the level of "what does each line of the code do", this is a fix, but
>> as we do a lot more than just stripping "[PATCH] " from the beginning of
>> the Subject: line these days, I think we are better off declaring defeat
>> in this particular codepath and not doing anything here.
>
> Sounds fine to me.
> Glad you're keeping everything in perspective.
What the case statement tries to do is _wrong_; "mailinfo -k" keeps the
original prefix and all the case statement does is to add an extra [PATCH]
that did not exist anywhere in the original on top of that.
What is funny is that the case statement has been trying to do a wrong
thing from day-one, ever since the script was introduced in d1c5f2a (Add
git-am, applymbox replacement., 2005-10-07). That version uses $keep to
hold -k or empty, gives that to mailinfo for producing $dotest/info, and
it has the same case statement that switches on $keep_subject nobody sets
to add an extra "[PATCH]" in front. Luckily, due to the typo you found,
nobody was bitten by the bug, and your patch will break things for people
by enabling it ;-).
Thanks for noticing this one. It began an innocent bug nobody noticed,
but it is embarrassing that we carefully _maintained_ that code nobody
triggers for four years.
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 11/11] mingw: compile git-daemon
From: Johannes Sixt @ 2009-11-27 21:17 UTC (permalink / raw)
To: msysgit; +Cc: Erik Faye-Lund, git, dotzenlabs, Erik Faye-Lund
In-Reply-To: <1259196260-3064-12-git-send-email-kusmabite@gmail.com>
On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
> --- a/Makefile
> +++ b/Makefile
> @@ -352,6 +352,7 @@ EXTRA_PROGRAMS =
>
> # ... and all the rest that could be moved out of bindir to gitexecdir
> PROGRAMS += $(EXTRA_PROGRAMS)
> +PROGRAMS += git-daemon$X
> PROGRAMS += git-fast-import$X
> PROGRAMS += git-hash-object$X
> PROGRAMS += git-imap-send$X
> @@ -981,6 +982,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
> NO_REGEX = YesPlease
> BLK_SHA1 = YesPlease
> + NO_INET_PTON = YesPlease
> + NO_INET_NTOP = YesPlease
> COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch
> COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
> # We have GCC, so let's make use of those nice options
> @@ -1079,9 +1082,6 @@ ifdef ZLIB_PATH
> endif
> EXTLIBS += -lz
>
> -ifndef NO_POSIX_ONLY_PROGRAMS
> - PROGRAMS += git-daemon$X
> -endif
> ifndef NO_OPENSSL
> OPENSSL_LIBSSL = -lssl
> ifdef OPENSSLDIR
You should remove NO_POSIX_ONLY_PROGRAMS from MSVC and MinGW sections.
-- Hannes
^ permalink raw reply
* [PATCH] t7508-status.sh: Add tests for status -s
From: Michael J Gruber @ 2009-11-27 21:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
The new short status has been completely untested so far. Introduce
tests by duplicating all tests which are present for the long format.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
t/t7508-status.sh | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 166 insertions(+), 0 deletions(-)
I think having them one by one (long, short, long, short, ...) makes more sense
than having them in separate files. But I'm not sure I should follow this
strategy when testing with color and with --porcelain as well. Maybe
it's enough to duplicate a few tests only for --porcelain and with
color?
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 1173dbb..99a74bd 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -68,6 +68,24 @@ test_expect_success 'status (2)' '
'
+cat > expect << \EOF
+ M dir1/modified
+A dir2/added
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+
+test_expect_success 'status -s (2)' '
+
+ git status -s > output &&
+ test_cmp expect output
+
+'
+
cat >expect <<EOF
# On branch master
# Changes to be committed:
@@ -97,6 +115,22 @@ test_expect_success 'status (status.showUntrackedFiles no)' '
test_cmp expect output
'
+cat >expect << EOF
+ M dir1/modified
+A dir2/added
+EOF
+test_expect_success 'status -s -uno' '
+ git config --unset status.showuntrackedfiles
+ git status -s -uno >output &&
+ test_cmp expect output
+'
+
+test_expect_success 'status -s (status.showUntrackedFiles no)' '
+ git config status.showuntrackedfiles no
+ git status -s >output &&
+ test_cmp expect output
+'
+
cat >expect <<EOF
# On branch master
# Changes to be committed:
@@ -133,6 +167,29 @@ test_expect_success 'status (status.showUntrackedFiles normal)' '
'
cat >expect <<EOF
+ M dir1/modified
+A dir2/added
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? dir3/
+?? expect
+?? output
+?? untracked
+EOF
+test_expect_success 'status -s -unormal' '
+ git config --unset status.showuntrackedfiles
+ git status -s -unormal >output &&
+ test_cmp expect output
+'
+
+test_expect_success 'status -s (status.showUntrackedFiles normal)' '
+ git config status.showuntrackedfiles normal
+ git status -s >output &&
+ test_cmp expect output
+'
+
+cat >expect <<EOF
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
@@ -169,6 +226,29 @@ test_expect_success 'status (status.showUntrackedFiles all)' '
test_cmp expect output
'
+cat >expect <<EOF
+ M dir1/modified
+A dir2/added
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+test_expect_success 'status -s -uall' '
+ git config --unset status.showuntrackedfiles
+ git status -s -uall >output &&
+ test_cmp expect output
+'
+test_expect_success 'status -s (status.showUntrackedFiles all)' '
+ git config status.showuntrackedfiles all
+ git status -s >output &&
+ rm -rf dir3 &&
+ git config --unset status.showuntrackedfiles &&
+ test_cmp expect output
+'
+
cat > expect << \EOF
# On branch master
# Changes to be committed:
@@ -201,6 +281,23 @@ test_expect_success 'status with relative paths' '
'
cat > expect << \EOF
+ M modified
+A ../dir2/added
+?? untracked
+?? ../dir2/modified
+?? ../dir2/untracked
+?? ../expect
+?? ../output
+?? ../untracked
+EOF
+test_expect_success 'status -s with relative paths' '
+
+ (cd dir1 && git status -s) > output &&
+ test_cmp expect output
+
+'
+
+cat > expect << \EOF
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
@@ -232,6 +329,24 @@ test_expect_success 'status without relative paths' '
'
+cat > expect << \EOF
+ M dir1/modified
+A dir2/added
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+
+test_expect_success 'status -s without relative paths' '
+
+ (cd dir1 && git status -s) > output &&
+ test_cmp expect output
+
+'
+
cat <<EOF >expect
# On branch master
# Changes to be committed:
@@ -298,6 +413,28 @@ test_expect_success 'status --untracked-files=all does not show submodule' '
test_cmp expect output
'
+cat >expect <<EOF
+ M dir1/modified
+A dir2/added
+A sm
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+test_expect_success 'status -s submodule summary is disabled by default' '
+ git status -s >output &&
+ test_cmp expect output
+'
+
+# we expect the same as the previous test
+test_expect_success 'status -s --untracked-files=all does not show submodule' '
+ git status -s --untracked-files=all >output &&
+ test_cmp expect output
+'
+
head=$(cd sm && git rev-parse --short=7 --verify HEAD)
cat >expect <<EOF
@@ -335,6 +472,21 @@ test_expect_success 'status submodule summary' '
test_cmp expect output
'
+cat >expect <<EOF
+ M dir1/modified
+A dir2/added
+A sm
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+test_expect_success 'status -s submodule summary' '
+ git status -s >output &&
+ test_cmp expect output
+'
cat >expect <<EOF
# On branch master
@@ -365,6 +517,20 @@ test_expect_success 'status submodule summary (clean submodule)' '
'
cat >expect <<EOF
+ M dir1/modified
+?? dir1/untracked
+?? dir2/modified
+?? dir2/untracked
+?? expect
+?? output
+?? untracked
+EOF
+test_expect_success 'status -s submodule summary (clean submodule)' '
+ git status -s >output &&
+ test_cmp expect output
+'
+
+cat >expect <<EOF
# On branch master
# Changes to be committed:
# (use "git reset HEAD^1 <file>..." to unstage)
--
1.6.6.rc0.274.g71380
^ permalink raw reply related
* Re: [PATCH/RFC 2/2] Makefile: automatically compute header dependencies
From: Sverre Rabbelier @ 2009-11-27 22:57 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git Mailing List
In-Reply-To: <20091127175043.GC3461@progeny.tock>
Heya,
On Fri, Nov 27, 2009 at 18:50, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Good idea? Bad idea?
Ugh, git/git is already a horror to 'ls', adding another n files...
:(. Which brings me back to "if only git had a seperate src/ and maybe
/build directories" :P.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Sverre Rabbelier @ 2009-11-28 0:45 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, Johan Herland, git
In-Reply-To: <alpine.LNX.2.00.0911271417010.14365@iabervon.org>
Heya,
On Fri, Nov 27, 2009 at 20:17, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Thu, 26 Nov 2009, Sverre Rabbelier wrote:
>> On Thu, Nov 26, 2009 at 02:03, Junio C Hamano <gitster@pobox.com> wrote:
>> > * sr/vcs-helper (2009-11-18) 12 commits
>> > Replaced again, and looking good. Perhaps Daniel has some comments?
>>
>> Indeed, Johan, Daniel, is the current version good for next?
>
> Looks good to me.
Awesome, I'd say this is good for next then.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Unable to checkout a particular SVN revision
From: Marc Liyanage @ 2009-11-28 2:05 UTC (permalink / raw)
To: git
I'm trying to clone a specific SVN revision with git-svn:
git svn clone -r 12345 https://host/svn/foo/branches/bar xyz
but it doesn't check out any files, I see just this:
Initialized empty Git repository in /Users/liyanage/Desktop/xyz/.git
If I try the same thing with SVN like this:
svn co -r 12345 https://host/svn/foo/branches/bar xyz
then I get what I expect, it checks out all the files and "svn info" gives me this revision.
I think it's because this particular revision wasn't committed on this branch, i.e. it doesn't show up in "svn log". If I try a revision that is listed in the log, it works as expected.
Is there a way to make this work?
^ permalink raw reply
* Re: [PATCH/RFC 2/2] Makefile: automatically compute header dependencies
From: Jonathan Nieder @ 2009-11-28 4:24 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git Mailing List
In-Reply-To: <fabb9a1e0911271457k31d8addcwbbc8fd34f9aedd8c@mail.gmail.com>
Sverre Rabbelier wrote:
> Ugh, git/git is already a horror to 'ls', adding another n files...
> :(.
They are dotfiles, though depending on how your 'ls' works, that may or
may not help.
> Which brings me back to "if only git had a seperate src/ and maybe
> /build directories" :P.
Hmm, I don’t want to work on that in general, but a separate deps/
directory does not sound like a bad idea at all.
i.e., something vaguely like this.
.gitignore | 1 +
Makefile | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 2 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 ed0f461..1cc149b 100644
--- a/Makefile
+++ b/Makefile
@@ -488,6 +488,7 @@ LIB_H += unpack-trees.h
LIB_H += userdiff.h
LIB_H += utf8.h
LIB_H += wt-status.h
+LIB_H :=
LIB_OBJS += abspath.o
LIB_OBJS += advice.o
@@ -1559,13 +1560,23 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+
%.o: %.c GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
%.o: %.S
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+objects := $(wildcard *.o block-sha1/*.o ppc/*.o compat/*.o \
+ compat/*/*.o xdiff/*.o)
+dep_files := $(wildcard $(foreach f,$(objects),$(dir $f)deps/$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
exec_cmd.o: exec_cmd.c GIT-CFLAGS
exec_cmd.o: ALL_CFLAGS += \
'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
@@ -1657,6 +1668,9 @@ TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
$(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
GIT-CFLAGS: .FORCE-GIT-CFLAGS
+ mkdir -p deps block-sha1/deps ppc/deps compat/deps \
+ compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
+ xdiff/deps
@FLAGS='$(TRACK_CFLAGS)'; \
if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
echo 1>&2 " * new build flags or prefix"; \
@@ -1873,8 +1887,10 @@ distclean: clean
$(RM) configure
clean:
- $(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+ $(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
$(LIB_FILE) $(XDIFF_LIB)
+ $(RM) -r deps block-sha1/deps ppc/deps compat/deps \
+ compat/*/deps xdiff/deps
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
--
1.6.5.3
^ permalink raw reply related
* Re: [PATCH v3] Give the hunk comment its own color
From: Junio C Hamano @ 2009-11-28 5:52 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Jeff King, git
In-Reply-To: <1259304918-12600-1-git-send-email-bert.wesarg@googlemail.com>
Bert Wesarg <bert.wesarg@googlemail.com> writes:
> diff.c | 64 +++++++++++++++++++++++++++++++++++++++++++--
> ...
> @@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,
> }
> }
>
> +static void emit_hunk_line(struct emit_callback *ecbdata,
> + const char *line, int len)
> +{
> + const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
> + const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
> + const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
> + const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
> + const char *orig_line = line;
> + int orig_len = len;
> + const char *frag_start;
> + int frag_len;
> + const char *part_end = NULL;
> + int part_len = 0;
> +
> + /* determine length of @ */
> + while (part_len < len && line[part_len] == '@')
> + part_len++;
> +
> + /* find end of frag, (Ie. find second @@) */
> + part_end = memmem(line + part_len, len - part_len,
> + line, part_len);
This is not incorrect per-se, but probably is overkill; this codepath only
deals with two-way diff and we know we are looking at "@@ -..., +... @@"
at this point.
part_end = memmem(line + 2, len - 2, "@@", 2);
would be sufficient.
> + if (!part_end)
> + return emit_line(ecbdata->file, frag, reset, line, len);
> + /* calculate total length of frag */
> + part_len = (part_end + part_len) - line;
> +
> + /* remember frag part, we emit only if we find a space separator */
> + frag_start = line;
> + frag_len = part_len;
> +
> + /* consume hunk header */
> + len -= part_len;
> + line += part_len;
> +
> + /*
> + * for empty reminder or empty space sequence (exclusive any newlines
> + * or carriage returns) emit complete original line as FRAGINFO
> + */
> + if (!len || !(part_len = strspn(line, " \t")))
Slightly worrisome is what guarantees this strspn() won't step outside
len.
I would probably write the function like this instead.
-- >8 --
static void emit_hunk_header(struct emit_callback *ecbdata,
const char *line, int len)
{
const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
static const char atat[2] = { '@', '@' };
const char *cp, *ep;
/*
* As a hunk header must begin with "@@ -<old>, +<new> @@",
* it always is at least 10 bytes long.
*/
if (len < 10 ||
memcmp(line, atat, 2) ||
!(ep = memmem(line + 2, len - 2, atat, 2))) {
emit_line(ecbdata->file, plain, reset, line, len);
return;
}
ep += 2; /* skip over the second @@ */
/* The hunk header in fraginfo color */
emit_line(ecbdata->file, frag, reset, line, ep - line);
/* blank before the func header */
for (cp = ep; ep - line < len; ep++)
if (*ep != ' ' && *ep != 't')
break;
if (ep != cp)
emit_line(ecbdata->file, plain, reset, cp, ep - cp);
if (ep < line + len)
emit_line(ecbdata->file, func, reset, ep, line + len - ep);
}
^ permalink raw reply
* [PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2009-11-28 9:29 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Sverre Rabbelier, Johannes Sixt, Junio C Hamano, Jeff King,
Git Mailing List
In-Reply-To: <20091127175043.GC3461@progeny.tock>
Use the gcc -MMD -MP -MF options to generate dependencies as a
byproduct of building .o files.
This feature has to be optional (I don’t think MSVC, for example,
supports anything like this), so unless someone hooks in a rule
to check the static header dependencies for correctness, this
won’t help much with header dependency maintainance. It is
enabled by setting the COMPUTE_HEADER_DEPENDENCIES variable,
unset by default.
The scope of the %.o: %.c pattern rule has been restricted to
make it easier to tell if a new object file has not been hooked
into the dependency generation machinery.
An unrelated fix also snuck in: the %.s: %.c pattern rule to
generate an assembler listing did not have correct dependencies.
It is meant to be invoked by hand and should always run.
To avoid litering the build directory with even more build
products, the generated Makefile fragments are squirreled away
into deps/ subdirectories of each directory containing object
files. These directories are currently generated as a
side-effect of the GIT-CFLAGS rule, to guarantee they will be
available whenever the %.o: %.c and %.o: %.S pattern rules are
being used. This is really not ideal, especially because it
requires hard-coding the list of directories with objects.
gcc learned the -MMD -MP -MF options in version 3.0, so most gcc
users should have them by now.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I’ll send the .c.s and dependency fixes separately.
Thoughts? Advice?
Thanks,
Jonathan
.gitignore | 1 +
Makefile | 63 ++++++++++++++++++++++++++++++++++++++++++-----------------
2 files changed, 46 insertions(+), 18 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 ed0f461..fb20302 100644
--- a/Makefile
+++ b/Makefile
@@ -216,6 +216,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-GIT-VERSION-FILE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1559,12 +1562,42 @@ git.o git.spec \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
: GIT-VERSION-FILE
-%.o: %.c GIT-CFLAGS
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS
+GIT_OBJS := http.o http-walker.o http-push.o \
+ $(LIB_OBJS) $(BUILTIN_OBJS) \
+ $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o
+
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
+
+ifndef COMPUTE_HEADER_DEPENDENCIES
+$(GIT_OBJS): $(LIB_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
+
+http.o http-walker.o http-push.o: http.h
+
+builtin-revert.o wt-status.o: wt-status.h
+
+$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
+else
+dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d))
+
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
+# Take advantage of gcc's dependency generation.
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+dep_args = -MF $(dep_file) -MMD -MP
+dep_file = $(dir $@)deps/$(notdir $@).d
+endif
+
+$(OBJECTS): %.o: %.c GIT-CFLAGS
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+%.s: %.c GIT-CFLAGS .FORCE-LISTING
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S
- $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(OBJECTS): %.o: %.S GIT-CFLAGS
+ $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
exec_cmd.o: exec_cmd.c GIT-CFLAGS
exec_cmd.o: ALL_CFLAGS += \
@@ -1594,10 +1627,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)
@@ -1609,22 +1638,15 @@ 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)
-
doc:
$(MAKE) -C Documentation all
@@ -1657,6 +1679,9 @@ TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
$(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
GIT-CFLAGS: .FORCE-GIT-CFLAGS
+ mkdir -p deps block-sha1/deps ppc/deps compat/deps \
+ compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
+ xdiff/deps
@FLAGS='$(TRACK_CFLAGS)'; \
if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
echo 1>&2 " * new build flags or prefix"; \
@@ -1873,8 +1898,10 @@ distclean: clean
$(RM) configure
clean:
- $(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+ $(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
$(LIB_FILE) $(XDIFF_LIB)
+ $(RM) -r deps block-sha1/deps ppc/deps compat/deps \
+ compat/*/deps xdiff/deps
$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
@@ -1899,7 +1926,7 @@ endif
.PHONY: all install clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
### Check documentation
#
--
1.6.5.3
^ permalink raw reply related
* Re: [PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies
From: Andreas Schwab @ 2009-11-28 9:26 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Schindelin, Sverre Rabbelier, Johannes Sixt,
Junio C Hamano, Jeff King, Git Mailing List
In-Reply-To: <20091128092948.GA8515@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> GIT-CFLAGS: .FORCE-GIT-CFLAGS
> + mkdir -p deps block-sha1/deps ppc/deps compat/deps \
> + compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
> + xdiff/deps
IMHO the list of directories should be factored out in a variable for
easier maintenance.
> @@ -1873,8 +1898,10 @@ distclean: clean
> $(RM) configure
>
> clean:
> - $(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
> + $(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
> $(LIB_FILE) $(XDIFF_LIB)
> + $(RM) -r deps block-sha1/deps ppc/deps compat/deps \
> + compat/*/deps xdiff/deps
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH 0/4] Makefile fixes
From: Jonathan Nieder @ 2009-11-28 11:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Here are the aforementioned small fixes for the Makefile, intended for
maint. I hope they are of some use.
Jonathan Nieder (4):
Makefile: http-push.c uses the git headers
Makefile: make ppc/sha1ppc.o depend on GIT-CFLAGS
Makefile: fix .s pattern rule dependencies
Makefile: stop cleaning arm directory
Makefile | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/2] format-patch: fix dashdash usage
From: Felipe Contreras @ 2009-11-28 11:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbsg25sb.fsf@alter.siamese.dyndns.org>
On Fri, Nov 27, 2009 at 2:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
> (0) take your patch with an updated message (eh, that is not "next step"
> but the "first step");
I guess it depends of what is the "fist step". To me the "first step"
is realizing there's a problem. First patch submission is already many
steps afterwards.
But sure, I'll do this next step :)
> (1) make --full-diff implicit and default of format-patch (--no-full-diff
> could be supported _if_ somebody can argue successfully why limiting
> the diff is also a useful thing to do); and
I started writing this patch but it turns out there's no --no-full-diff.
> (2) document clearly that format-patch takes optional pathspecs, and in
> what situation they are useful.
I guess this would have to be done in multiple places so it might take
me some time.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [PATCH 1/4] Makefile: fix http-push.o dependencies
From: Jonathan Nieder @ 2009-11-28 11:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091128112546.GA10059@progeny.tock>
Since it is not in LIB_OBJS, http-push.o needs an explicit
$(GIT_H) dependency.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
http-push.c begins:
#include "cache.h"
#include "commit.h"
Makefile | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 856ba09..dc7c929 100644
--- a/Makefile
+++ b/Makefile
@@ -1557,9 +1557,7 @@ 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)
+http.o http-walker.o http-push.o: http.h $(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,$^) \
--
1.6.5.3
^ permalink raw reply related
* [PATCH 2/4] Makefile: make ppc/sha1ppc.o depend on GIT-CFLAGS
From: Jonathan Nieder @ 2009-11-28 11:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091128112546.GA10059@progeny.tock>
Any rule that makes use of ALL_CFLAGS should depend on GIT-CFLAGS
to avoid trouble. This one would not actually be affected by any
build flags except the optimization level, so leaving the
dependency out was mostly harmless.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index dc7c929..bb3879e 100644
--- a/Makefile
+++ b/Makefile
@@ -1526,7 +1526,7 @@ git.o git.spec \
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
%.s: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S
+%.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
exec_cmd.o: exec_cmd.c GIT-CFLAGS
--
1.6.5.3
^ permalink raw reply related
* [PATCH 3/4] Makefile: fix .s pattern rule dependencies
From: Jonathan Nieder @ 2009-11-28 11:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20091128112546.GA10059@progeny.tock>
'make git.s' fails to regenerate an assembler listing if git.c
has not changed but a header it includes has. The %.s: %.c
pattern rule is meant to be invoked by hand, so it would be
better to make it always run.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This adds yet another phony .FORCE-foo target. Wouldn’t it be simpler
to use a single target called .FORCE, or is there something I am
missing that that would break?
Makefile | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index bb3879e..cd210e3 100644
--- a/Makefile
+++ b/Makefile
@@ -1524,7 +1524,7 @@ git.o git.spec \
%.o: %.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS
+%.s: %.c GIT-CFLAGS .FORCE-LISTING
$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
%.o: %.S GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
@@ -1859,7 +1859,7 @@ endif
.PHONY: all install clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
### Check documentation
#
--
1.6.5.3
^ 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