* 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 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: [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: [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: [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: [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 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] 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: [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: [msysGit] [PATCH/RFC 06/11] run-command: add kill_async() and is_async_alive()
From: Johannes Sixt @ 2009-11-27 19:59 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <40aa078e0911270804i1a828ea6we1611047d37869f7@mail.gmail.com>
On Freitag, 27. November 2009, Erik Faye-Lund wrote:
> Do you really think it's better to unconditionally take down the
> entire process with an error, instead of having a relatively small
> chance of stuff blowing up without any sensible error? I'm not 100%
> convinced - but let's hope we'll find a proper fix.
"relatively small chance of stuff blowing up"? The docs of
TerminateThread: "... the kernel32 state for the thread's process could be
inconsistent." That's scary if we are talking about a process that should run
for days or weeks without interruption.
The reason why we are killing a thread is to prevent keeping lots of
connections open (to the same IP address). There are two situations to take
care of:
1. We are in a lengthy computation without paying attention to the socket.
2. The client does not send or accept data for a long time.
Case 1 could happen if upload-pack is "counting objects" on a large
repository. We would need some way to kill upload-pack. Since it is a
separate process anyway, we could use TerminateProcess().
Case 2 could be achieved by using setsockopt() with SO_RCVTIMEO and
SO_SNDTIMEO and a tiny timeout. But notice that we would set a timeout in one
thread while another thread is waiting in ReadFile() or WriteFile(). Would
that work?
-- Hannes
^ permalink raw reply
* gitk: how to display history w/o a certain remote?
From: Dirk Süsserott @ 2009-11-27 19:52 UTC (permalink / raw)
To: Git Mailing List
Hi list,
next to those cute little spherules, gitk shows the labels of the
corresponding tags and branches, e.g. "master", "remotes/XXX/master",
and "remotes/YYY/master". Most of the time, I like that. :-)
"git log [...] --decorate" does it alike.
Sometimes I have many remotes and the actual commit message is far
beyond my screen because of the many labels.
Is there a way to prevent gitk/git-log from displaying certain remotes?
I tried the "--not" switch but that didn't work. At least for my arguments.
AHA,
Dirk
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Daniel Barkalow @ 2009-11-27 19:17 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, Johan Herland, git
In-Reply-To: <fabb9a1e0911251715u661ce0aem79a4d700d552e105@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1055 bytes --]
On Thu, 26 Nov 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Thu, Nov 26, 2009 at 02:03, Junio C Hamano <gitster@pobox.com> wrote:
> > * sr/vcs-helper (2009-11-18) 12 commits
> > - Add Python support library for remote helpers
> > - Basic build infrastructure for Python scripts
> > - Allow helpers to report in "list" command that the ref is unchanged
> > - Fix various memory leaks in transport-helper.c
> > - Allow helper to map private ref names into normal names
> > - Add support for "import" helper command
> > - Allow specifying the remote helper in the url
> > - Add a config option for remotes to specify a foreign vcs
> > - Allow fetch to modify refs
> > - Use a function to determine whether a remote is valid
> > - Allow programs to not depend on remotes having urls
> > - Fix memory leak in helper method for disconnect
> >
> > Replaced again, and looking good. Perhaps Daniel has some comments?
>
> Indeed, Johan, Daniel, is the current version good for next?
Looks good to me.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [msysGit] [PATCH/RFC 03/11] mingw: implement syslog
From: Johannes Sixt @ 2009-11-27 19:23 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, git, dotzenlabs
In-Reply-To: <40aa078e0911270009u7569cfe5gb250092c8d2c0eac@mail.gmail.com>
On Freitag, 27. November 2009, Erik Faye-Lund wrote:
> On Thu, Nov 26, 2009 at 10:23 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> > I would
> >
> > const char* arg;
> > if (strcmp(fmt, "%s"))
> > die("format string of syslog() not implemented")
> > va_start(va, fmt);
> > arg = va_arg(va, char*);
> > va_end(va);
> >
> > because we have exactly one invocation of syslog(), which passes "%s" as
> > format string. Then strbuf_vaddf is not needed. Or even simpler: declare
> > the function as
> >
> > void syslog(int priority, const char *fmt, const char*arg);
>
> After reading this again, I agree that this is the best solution. I'll
> update for the next iteration.
Except that you shouldn't die like I proposed because here we are already in
the die_routine, no?
> > "Note that the string that you log cannot contain %n, where n is an
> > integer value (for example, %1) because the event viewer treats it as an
> > insertion string. ..."
> >
> > How are the chances that this condition applies to our use of the
> > function?
>
> Ugh, increasingly high since we're adding IPv6 support, I guess.
> Perhaps some form of escaping needs to be done?
I think so, but actually I have no clue.
-- Hannes
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Uri Okrent @ 2009-11-27 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Jeff King, James Pickens, git
In-Reply-To: <7vk4xbn7nl.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> You preach to the choir.
>
> That is exactly how we work and what people have been working hard for
> 1.7.0. Check the planned changes listed in the recent (and not so recent)
> "What's cooking" summary reports.
Yes, I guess my only point here was that maybe even 1.7 is not enough of
a "Big Deal" (in the eyes of the public) to warrant breaking scripts. A
2.0 version would be a more visible way to say "Hey test your scripts
before upgrading". Adopting a strategy like that would mean making
backwards incompatible changes a lot let frequently, but when we do we
go for broke.
> 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.
--
Uri
Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Jonathan Nieder @ 2009-11-27 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vvdgvn7ny.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> The flipping of "grep" default is too late for 1.7.0, as we have only
> started discussing it, and I want the release after 1.6.6 to be 1.7.0.
Yes, I’m sorry I suggested it. I think you had mentioned this plan
before, but somehow it escaped my mind.
^ permalink raw reply
* Re: OS X and umlauts in file names
From: Thomas Singer @ 2009-11-27 18:35 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Jay Soffian, Daniel Barkalow, git
In-Reply-To: <46a038f90911270256w57487e6cq9bda8b98a5384799@mail.gmail.com>
Martin Langhoff wrote:
> have you tried calling git-update-index --add
> --stdin -z? Your original email stated
No, we don't do such a massive change immediately before a release.
>> we've got a problem report regarding our SmartGit GUI client
>
> so it sounds like you are building a porcelain. In that case, the
> sanest approach is to invoke git-update-index and write to its stdin.
We will try this out after release.
For those who are interested: I've got it working on OS X and Git was not
the problem, but Java. A longer time ago directory.list() or
directory.listFiles() returned the file names with decomposed characters (as
they are stored on OS X hard disk). Now (don't know which Java update
introduced this change) these methods return file names with composed
characters, so I had to decompose them before handing them to the git
executable call.
Nevertheless, the cross-platform-problem remains: if you add files with
umlauts in their names on non-OS X, you will not be able to use them on OS X.
--
Tom
^ permalink raw reply
* Re: [RFC/PATCH] gitweb: Make linking to actions requiring JavaScript a feature
From: Junio C Hamano @ 2009-11-27 18:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Stephen Boyd, git, Martin Koegler
In-Reply-To: <200911271641.40947.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Would turning
>
> "blame"
>
> link ito pair of links
>
> "blame (incremental)"
>
> be a good solution? I'm trying to come up with good naming for extra link
> to 'blame_incremental' action...
I had to step back a bit and ask myself what we are trying to achieve
here. When the current blame and incremental one are both working
perfectly and well, will there be a reason for the end users to choose
between them when they click?
My answer is no. If the incremental one gives nicer user experience in
all cases, it will be shown without the non-incremental one; if the
incremental one makes the server or network load too heavy, a site owner
may decide to show only the non-incremental one.
That makes my addLinks suggestion a change that would help _only_ while we
are working kinks out of the incremental one.
Let's not waste too much effort doing that. Sorry for suggesting.
Letting the site owner choose if the site wants to set the "incremental if
possible" boolean would be more than adequate, I think.
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Junio C Hamano @ 2009-11-27 18:29 UTC (permalink / raw)
To: Uri Okrent; +Cc: Johannes Schindelin, Jeff King, James Pickens, git
In-Reply-To: <6839293b0911270827x54947c64q5f93e37664bc20f3@mail.gmail.com>
Uri Okrent <uokrent@gmail.com> writes:
> The key once again, is managing expectations. We can't go around
> changing everything willy-nilly, and we can't be continually changing
> things. Here is where we could take a lesson from the python
> community.
>
> When they decided they needed to change things, they bundled a
> bunch of backwards incompatible changes together and went for it.
> Yes, Python 3 will break your scripts, but the most important thing is,
> everybody knows it.
>
> A similar thing was done here with the huge warning that push spits
> out, but in the general case I would argue, that the wisest course is to
> save backwards incompatible changes for a git 2 or something, where
> we know we're breaking the world, and then scratch all our (well thought
> out) backwards incompatible itches at once.
You preach to the choir.
That is exactly how we work and what people have been working hard for
1.7.0. Check the planned changes listed in the recent (and not so recent)
"What's cooking" summary reports.
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.
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Junio C Hamano @ 2009-11-27 18:29 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, James Pickens, git
In-Reply-To: <20091127095914.GA4865@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Nov 27, 2009 at 10:31:30AM +0100, Johannes Schindelin wrote:
>
>> Guess what. I have a similar problem, only it is that my "git status"
>> output is _always_ too long, so I always have to page it.
>>
>> Once upon a time, Junio applied a patch that implied -p with status. I
>> was overjoyed. He reverted that patch later. Yes, exactly.
It would have been more fair to me if Dscho said "He had to revert", to
hint that it was not due to me changing the preference left and right on a
whim.
>> So I end up doing "git config --global ps '-p status'" on every new
>
> 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.
A "status" command that pages or does not page the output only when
spitting out to a terminal won't hurt somebody who helps another with the
configuration set differently, as much as a "grep" that shows or not shows
matches from other parts of the tree would, and when used by a script to
make a decision based on the output, the caller has to capture the output
first, and unless the caller drives "status" via pty, e.g. using "expect",
paging behaviour will be disabled no matter what the configuration setting
is.
So neither "it would hurt people who help others" nor "it would hurt
scripts" would apply to "status". But both would apply to "grep".
>> The further benefit is that we stop talking about breaking backwards
>> compatibility, and we stop talking about making it hard for Git experts to
>> help newbies.
>
> I guess you missed the part of the thread where I already discussed
> this. It was here:
>
> http://article.gmane.org/gmane.comp.version-control.git/133672
This was a very good summary, and was one of the reasons that made me
reconsider placing too much weight on "it would hurt people who help
others" (but not on "it would hurt scripts").
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Junio C Hamano @ 2009-11-27 18:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jeff King, git
In-Reply-To: <20091127144553.GA24366@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
>> I can try to pick this up. But did we reach a decision on having a
>> configuration variable?
>
> I am not sure, but I will say I would prefer not to have one. Surely
> we can come up with a UI that does not require searching through
> git-config(1) to be made convenient.
>
> Couldn’t we just add the option (with test and documentation) first,
> to get some experience with how we end up using the two forms?
>
> If --full-tree does become the default, I think it should be in 1.7.0,
> when it is expected for some habits to break (with a configuration
> variable for the transition, I guess). This might be okay, since
> constructions like 'git grep foo -- "./*.h"' should still work.
The flipping of "grep" default is too late for 1.7.0, as we have only
started discussing it, and I want the release after 1.6.6 to be 1.7.0.
I do not think we want nor need to have other things we have already
prepared and have been advertising for 1.7.0 to wait one more cycle.
^ permalink raw reply
* Re: [PATCH] Makefile: determine the list of header files using a glob
From: Junio C Hamano @ 2009-11-27 18:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.0911271033460.4521@intel-tinevez-2-302>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Sidestep the issue by computing the list using $(wildcard).
>
> Funny; I thought that not all header files are library header files, i.e.
> not all header changes should trigger a full new build of libgit.a.
>
> Am I wrong?
You are right.
^ permalink raw reply
* Re: [PATCH] Makefile: determine the list of header files using a glob
From: Junio C Hamano @ 2009-11-27 18:28 UTC (permalink / raw)
To: Mike Hommey; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <20091127085802.GA21217@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> On Fri, Nov 27, 2009 at 09:50:47AM +0100, Johannes Sixt wrote:
>> Mike Hommey schrieb:
>> > I don't know if the current Makefile works with Solaris' make,...
>>
>> No, it doesn't. You have to use GNU make anyway.
>
> Then it's fine. But shouldn't that be noted somewhere, like in the
> INSTALL file ?
Surely. Please make it so.
^ permalink raw reply
* Re: Breaking expectations in 1.7.0, was Re: What's cooking in git.git (Nov 2009, #06; Wed, 25)
From: Jonathan Nieder @ 2009-11-27 18:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jeff King
In-Reply-To: <20091127153349.GA24647@progeny.tock>
Hi again,
I wrote:
> I was trained by 1.6.0 and 1.5.0, I guess.
This misses the point. The big difference is that 1.7.0 is not that far
away, so it is not the time to tack on world-shaking changes.
So please substitute 1.8.0 or 1.9.0 for 1.7.0 in my message to Peff.
The point doesn’t change, though, which is that I think it makes no
sense to add a configuration variable for this before changing the
default.
Thanks for the clarification,
Jonathan
^ permalink raw reply
* Re: [PATCH] grep: --full-tree
From: Jeff King @ 2009-11-27 18:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: James Pickens, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0911271144230.4521@intel-tinevez-2-302>
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?
> 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.
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.
2. Some of these setups will be using the new config option.
If they are all controlled by a single user, how is that user any worse
off for the config option existing? They can choose not to use it if
the hassle is not worth it. I do not think the existence of an option is
giving too much rope to these users.
If you are talking about 10 machines, all controlled by different users,
whose terminals you have to sit down on to help them, then yes, it will
be inconvenient for you. But if users are setting up configuration for
these machines, shouldn't _their_ convenience in using configuration
trump _your_ convenience for occasionally sitting down and helping them?
> I, for one, do not like Git's reputation, but I am tired of trying to
> fight for the users. BTW quick question: how many Git _users_ were at the
> GitTogether at MV? 0?
In my opinion, you are actively fighting _against_ a user in this case.
And the GitTogether had a "users complain about git, and we try to
listen" session. There were two google users in person, but we also went
through a list of pre-made questions from other googlers. This issue
wasn't discussed, though. Nor was the question of consistency between
configurations, to my recollection. I think Shawn may have taken notes,
and could be more specific.
> > http://article.gmane.org/gmane.comp.version-control.git/133672
>
> I only skimmed it, yes. And I did not plan to participate in this thread.
> But it seems that my views are not represented enough, even if gitzilla
> chimed in with the very valid, under-acknowledged and over-ignored
> message: consistency is good. Corollary: inconsistency is bad.
That is an over-simplification. Inconsistency between setups is bad. But
so is inconsistency between git commands, and between git and other
commands. So is not supporting a user's workflow, or supporting it in a
way that is tedious and error-prone. You have to weigh the badness of
those things against each other in finding a solution.
But then, that was the point I already made in the article linked above.
-Peff
^ permalink raw reply
* [PATCH/RFC 2/2] Makefile: automatically compute header dependencies
From: Jonathan Nieder @ 2009-11-27 17:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, Git Mailing List
In-Reply-To: <20091127174558.GA3461@progeny.tock>
Use the gcc -MMD -MP -MF options to generate dependency rules as a
byproduct when building .o files.
A bit remains to be done:
- add the same support to the .c.s rule
- make this optional (not all compilers support this, and not all
developers necessarily want to litter the directory with .*.o.d
files)
- document what gcc version introduced these options
- find equivalent options for other compilers (e.g., Intel C,
SunWSPro, MSVC)
but this should give the idea.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Good idea? Bad idea?
Good night,
Jonathan
.gitignore | 1 +
Makefile | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/.gitignore b/.gitignore
index ac02a58..c7b2736 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..af3f874 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 $@).$(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 arm/*.o ppc/*.o \
+ compat/*.o compat/*/*.o xdiff/*.o)
+dep_files := $(wildcard $(foreach f,$(objects),$(dir $f).$(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)"' \
@@ -1875,6 +1886,8 @@ distclean: clean
clean:
$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
$(LIB_FILE) $(XDIFF_LIB)
+ $(RM) .*.o.d block-sha1/.*.o.d arm/.*.o.d ppc/.*.o.d compat/.*.o.d \
+ compat/*/.*.o.d xdiff/.*.o.d
$(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
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