* Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)
From: Jakub Narebski @ 2010-01-26 1:49 UTC (permalink / raw)
To: J.H.; +Cc: Petr Baudis, git, John 'Warthog9' Hawley
In-Reply-To: <4B5DFFE5.6060908@eaglescrag.net>
On Mon, 25 Jan 2010, J.H. wrote:
> On 01/25/2010 05:56 AM, Petr Baudis wrote:
>> On Mon, Jan 25, 2010 at 02:48:26PM +0100, Jakub Narebski wrote:
>>> Now those patches (mine and J.H. both) make gitweb use locking
>>> (it is IIRC configurable in J.H. patch) to make only one process
>>> generate the page if it is missing from cache, or is stale. Now
>>> if it is missing, we have to wait until it is generated in full
>>> before being able to show it to client. While it is possible to
>>> "tee" output (using PerlIO::tee, or Capture::Tiny, or tie like
>>> CGI::Cache) writing it simultaneously to browser and to cache for
>>> the process that is generating data, it is as far as I understand
>>> it impossible for processes which are waiting for data. Therefore
>>> the need for "Generating..." page, so the user does not think that
>>> web server hung or something, and is not generating output.
>>
>> Ah, ok, so the message is there to cover up for a technical problem. ;-)
>> I didn't quite realize. Then, it would be great to tweak the mechanisms
>> so that the user does not really have to wait.
>
> No, that is an incorrect assumption on how the 'Generating...' page
> works, and your missing a bit of the point.
>
> (1) The message itself 'Generating...' is a cue to the user that
> something is happening and that the browser is not actually hanging.
> Web users are at the point where if things are not instantaneous and
> show immediately they will either browse away completely or hit the
> refresh button incessantly until content does appear. While the page is
> usually only seen for about a second, and I'll admit it can be annoying,
> it's nothing more than a 'sit tight a second'. For things like the
> front page it can take upwards of 7 seconds to generate for a single
> user, a lot to ask for a no response scenario.
Pasky is asking later whether instead of separate "Generating..." page
(be it server side using trick with http-equiv refresh taking place when
page/connection is closed, or JavaScript Ajax-y one) couldn't gitweb
use streaming and have early parts of generated page (in particular the
page header and navigation bar) serve as activity indicator. I think
it is possible, if a bit more complicated, and such feature can be
introduced in a separate commit.
As to 'sit tight for a second': the code can be modified to print/show
"Generating..." page only if we are waiting more than second (or more
than half of second with utime from Time::HiRes) for the data. Pasky,
would such change address some of your concerns about "Generating..."
page as activity indicator?
>
> (2) It prevents the stampeding herd problem, which was very vehemently
> discussed 4 years ago by HPA and myself and roughly boils down to this:
>
> When a single user comes into the site, in particular the front page, it
> kicks off a process that will start to generate at it, causing a huge
> amount of git requests into individual repositories and a lot of disk
> i/o. A second user will then come in and the same requests will start
> to be done from the beginning again, and so on until you basically kill
> the machine because the disk i/o goes up enough that it can't ever
> service the requests fast enough.
As you can see from the way my proof of concept (of splitting) caching
series is split, I think that those two features are separate. Using
locking to have one writer, and protect against cache miss stampede
problem is one issue, and having "Generating..." page is another issue.
They are not orthogonal as they are now, as "Generating..." page depends
on locking to check if page got generated, but it could simply check
whether the cache file is present already.
>
> This does 2 things in the end:
>
> 1) means there's only 1 copy of the page ever being generated, thus
> meaning there isn't extraneous and dangerous disk i/o going on on the system
>
> 2) prevents a user from reporting to the website that it's broken by
> giving them a visual cue that things aren't broken.
1) is about locking ('expires_variance' can help there, too), 2) is about
"Generating..." but could be done with streaming: "tee"/"tail" solution.
>> So, I wonder about two things:
>>
>> (i) How often does it happen that two requests for the same page are
>> received? Has anyone measured it? Or is at least able to make
>> a minimally educated guess? IOW, isn't this premature optimization?
>
> For most pages, not many but it happens more often than you think. The
> data I have is much too old to be useful now but the front page could,
> at times, have up to 30 people waiting for it without caching. This is
> a very important patch believe it or not. For a site the size of
> kernel.org it cannot exist without this.
>
> But here's a quick stat, in 36 hours git.kernel.org has had
> 156099 accesses world wide or about 1.2 accesses a second.
>
> android.git.kernel.org, in the same time period has had 115818 accesses.
>
> If the first request takes 7 seconds to generate, by the time it's done
> there are now 3 additional requests running. If it again takes 7
> seconds to generate there are now another 3 requests running, etc. Very
> quickly you've got so much i/o running the box more or less is useless.
On the other hand do cache for front page (projects list) ever expire
so that there are no stale data to serve? So the issue with growing I/O
is I guess only with priming the cache.
BTW. I wonder how well the 'expires_variance' solution used in CHI would
protect against cache miss stampedes; that probably depends on the ratio
of variance to time it takes gitweb to generate the page.
>> (ii) Can't the locked gitwebs do the equivalent of tail -f?
>
> Not really going to help much, most of the gitweb operations won't
> output much of anything beyond the header until it's collected all of
> the data it needs anyway and then there will be a flurry of output.
Well, it might be enough to have headers as activity indicator... but
on the other hand users can think that server is broken if they see no
further data, as opposed to the "Generating..." page.
> It also means that this 'Generating...' page will only work for caching
> schemes that tail can read out of, which I'm not sure it would work all
> that well with things like memcached or a non-custom caching layer where
> we don't necessarily have direct access to the file being written to.
I don't think that caching interfaces such as CHI or Cache::Cache, or
individual caching engines such as Cache::Memcached or Cache::FastMmap
offer streaming access. OTOH we can read from the side as the data
is generated ("tee" to a temporary file, or something), but it is even
more complication.
>
> At least the way I had it (and I'll admit I haven't read through Jakub's
> re-working of my patches so I don't know if it's still there) is that
> with background caching you only get the 'Generating...' page if it's
> new or the content is grossly out of data. If it's a popular page and
> it's not grossly out of date it shows you the 'stale' data while it
> generates the new content in the background anyway, only locking you out
> when the new file is being written. Or at least that's how I had it.
It is there, i.e. gitweb would serve stale data if it can be found,
but it lacks 'too stale to serve as stale data' timeout/expire time.
I skipped this feature to get split series out of door faster.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv11 12/20] Builtin-ify git-notes
From: Johan Herland @ 2010-01-26 2:09 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, gitster
In-Reply-To: <780e0a6b1001211028t35a19e2la6bb92f8ba94a689@mail.gmail.com>
On Thursday 21 January 2010, Stephen Boyd wrote:
> On Sun, Jan 17, 2010 at 1:04 PM, Johan Herland <johan@herland.net> wrote:
> > +
> > + if (launch_editor(path, buf, NULL)) {
> > + fprintf(stderr, "Please supply the note
> > contents using" + " either -m or -F
> > option.\n"); + exit(1);
>
> die()?
Yes, thanks. Will be part of the next iteration.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)
From: Jakub Narebski @ 2010-01-26 2:30 UTC (permalink / raw)
To: J.H.; +Cc: Petr Baudis, git, John 'Warthog9' Hawley
In-Reply-To: <4B5E01F5.20402@eaglescrag.net>
On Mon, 25 Jan 2010, J.H. wrote:
> On 01/25/2010 05:48 AM, Jakub Narebski wrote:
>> On Mon, Jan 25, 2010, Petr Baudis wrote:
>>> On Mon, Jan 25, 2010 at 12:46:39PM +0100, Jakub Narebski wrote:
>>>> On Sun, 24 Jan 2010, Petr Baudis wrote:
>>
>>>>> I have stupid question, common to both the original patch and this
>>>>> RFC.
>>>>>
>>>>>> [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
>>>>>> regenerating cache (WIP)
>>>>>
>>>>> Just why is a "Generating..." page appropriate?
>>>>>
>>>>> I have to admit I hate it; can you please at least make it
>>>>> configurable? Why is it needed at all? It [...] confuses
>>>>> non-interactive HTTP clients [...]
>>
>>>> Second, gitweb can always check User-Agent header, and serve
>>>> "Generating..." page only to web browsers:
>>>>
>>>> unless (defined $cgi->user_agent() &&
>>>> $cgi->user_agent() =~ /\b(Mozilla|Opera)\b/i) {
>>>> return;
>>>> }
>>>>
>>>> or something like that.
>>>
>>> I'm not too happy with this. What about Safari? Opera? ELinks? There's a
>>> lot of web browsers.
>>
>> The "Mozilla" part would catch all "Mozilla compatibile" web browsers,
>> including Firefox (and other Gecko-based web browsers), Internet Explorer,
>> WebKit based browsers including Safari and Chrome and Konqueror.
>> The "Opera" part would catch Opera.
>> http://www.nczonline.net/blog/2010/01/12/history-of-the-user-agent-string/
>>
>> As to other web browsers like Elinks, Lynx, w3m, Dillo, etc.: the issue
>> is whether they honor '<meta http-equiv="refresh" content="0" />'.
>> I think it is better to stay on the safe side; it is not disaster if web
>> browser is not shown "Generating..." page where it could (but see
>> explanation below).
>
> Most of them do, that particular tag has been around for a long time and
> since it doesn't require Javascript to do the page refresh it's pretty
> much universal.
For this method to work there is additional assumption about web browser,
namely that it would follow refresh only after it gets page in full (only
when connection closes). But perhaps that is true for all web browsers
following http-equiv refresh.
>
> The problem is going to be with things like wget when someone wants to
> snag a binary file. This works fine if the file is already cached, but
> the user doesn't get what they are expecting if they get a blob that
> isn't the final file, but the html contents of the page. I don't know
> of any hint that things like wget would send to the server that you
> could switch based on, but it would be more or less the non-background
> caching state.
Well, the above solution would work as long as you don't run wget with
--user-agent.
[...]
>>> But if it will be possible to turn this off eventually, it's all your
>>> call whether to bother implementing it. :-)
>>
>> In my implementation it is (or rather would be) as simple as just
>> not passing 'generating_info' => \&git_generating_data_html in the
>> GitwebCache::SimpleFileCache constructor.
>
> At least in mine it was don't allow background caching. It would force
> everyone to wait on the one process that was actually generating
> content. But it means a few blank pages with a spinning working icon
> until the cache releases it's exclusive lock.
Theoretically at least having "Generating..." page is orthogonal to using
locking, to background serving and to serving stale data. In current
implementation those features are dependent, serving stale data on locking,
and "Generating..." also on locking.
I think that serving stale data should depend on background caching, as
otherwise we introduce inequality in treating process which regenerates
cache (and must wait without background caching) and rest of processes
(which are served stale data).
Certainly in final version of patch all this should be made configurable.
P.S. With background caching there is a question whether parent should
generate and child display stale or "Generating...", or whether parent
should display stale/activity and child (re)generate cache entry. Also
there is an issue whether parent should waitpid for child, would fiddling
with SIGCHLD handler be enough, or is it not needed.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: git status showing phantom modifications
From: Michael Ludwig @ 2010-01-26 9:14 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <4017EB11-4B29-4599-B19A-91BC39632BBF@xing.com>
Am 25.01.2010 um 12:43 schrieb Michael Ludwig:
> Reading up about core.fileMode in git-config(1), I think you've pointed
> me to the solution. I should probably just set it to false. If that doesn't
> solve the issue, I'll report back.
Back with more info:
git version 1.6.4.2
Before adding any core.filemode setting to ~/.gitconfig or .git/config:
michael@wladimir:/cygdrive/m/REPOS6.TECH.git/comp :-) git config -l
user.name=Michael Ludwig
user.email=...
core.repositoryformatversion=0
core.filemode=true <= filemode already present
core.bare=false
core.logallrefupdates=true
core.ignorecase=true
core.autocrlf=false
svn-remote.svn.ignore-paths=^(some|paths)
svn-remote.svn.url=svn://localhost
svn-remote.svn.fetch=:refs/remotes/git-svn
svn.authorsfile=authors.txt
My git status output after cd to subdir shows many files "modified". But
they aren't! Edited ~/.gitconfig to include core.filemode = true [sic].
Here's the diff for the git config output:
--- /home/michael/a 2010-01-25 20:36:53.140625000 +0100
+++ /home/michael/b 2010-01-25 20:37:01.421875000 +0100
@@ -4,6 +4,7 @@
alias.ci=checkin
alias.co=checkout
alias.br=branch
+core.filemode=true
core.repositoryformatversion=0
core.filemode=true
core.bare=false
We're seeing core.filemode appear *twice*. Why that?
michael@wladimir:/cygdrive/m/REPOS6.TECH.git/comp :-) git st
# On branch master
nothing to commit (working directory clean)
So it seems adding core.filemode = true has fixed the issue, or at least
covered the symptoms. Adding core.filemode = false did not cause the
phantom modifications to disappear. Note that from reading git-config(1),
I had rather assumed this latter setting to have the desired effect.
And something seems to be wrong here. Why are modifications flagged when
doing git status from a subdir but not from the topdir?
--
Michael.Ludwig (#) XING.com
^ permalink raw reply
* Re: [PATCH] git-gui: use themed tk widgets with Tk 8.5
From: Sverre Rabbelier @ 2010-01-26 9:17 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Shawn O. Pearce, msysgit
In-Reply-To: <878wbln0oa.fsf@users.sourceforge.net>
Heya,
On Tue, Jan 26, 2010 at 01:05, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> There is a screenshot of this running on Windows 7 at
> http://www.patthoyts.tk/screenshots/git-gui-win32.png
Wow, that looks a lot better than what we have currently.
> 26 files changed, 697 insertions(+), 453 deletions(-)
But that's a huge change, is there no way to make it smaller (spliti t
up in multiple patches), so that it's easier to review?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [Mesa3d-dev] mesa_7_7_branch -> master merges
From: Keith Whitwell @ 2010-01-26 10:59 UTC (permalink / raw)
To: tom fogal; +Cc: Jose Fonseca, git@vger.kernel.org, mesa3d-dev
In-Reply-To: <auto-000021766217@sci.utah.edu>
On Mon, 2010-01-25 at 11:04 -0800, tom fogal wrote:
> I think we've touched on a core git workflow issue here, and its likely
> others have hit this && have a solution, so I've added the git ML to
> the CC list.
>
> Git: the situation in this repo is a fast-moving master that is
> including many changes to internal interfaces. Stable branches just
> get bugfixes, and are periodically merged to master. However, the more
> the heads diverge, the more difficult it is for a bugfix to merge into
> the head. The major issue is that more experienced developers should
> really weigh in on these merges, because they tend to automagically
> undo some of the interface changes. Yet during such a delay, master
> inevitably moves, and the bugfixer has to do even more work to "redo"
> the merge (and potentially get more review!).
>
> Of course, if there are two bugfixers trying to make separate changes
> in the same time period, this gets worse.
>
> Is there a workflow that can solve this issue?
>
Speaking from the Mesa side, I think part of our problem is that it's
not easy to build the entire mesa tree, which means that the developer
doing the merge cannot even compile-test the result, meaning that many
trivial failures go unnoticed.
I'd argue that if we had a maximal mesa build target that compiled
*everything*, regardless of whether it produced drivers or not, we'd
have a much better chance of catching bogus merge droppings.
Despite Jose's valid concerns, I'd still argue that the situation we
have now is superior to what came before - where people were supposed to
be cherry-picking bugfixes but more likely they were forgotten or it
fell on Brian's shoulders to do manually.
Keith
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Johannes Schindelin @ 2010-01-26 10:59 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Johannes Sixt, Junio C Hamano
In-Reply-To: <201001252045.28778.robin.rosenberg@dewire.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 6471 bytes --]
Hi,
thanks for Cc:ing me this time.
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> måndagen den 25 januari 2010 18.34.01 skrev Johannes Schindelin:
>
> > On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> > > >From 37a74ccd395d91e5662665ca49d7f4ec49811de0 Mon Sep 17 00:00:00 2001
> > >
> > > From: Robin Rosenberg <robin.rosenberg@dewire.com>
> > > Date: Mon, 25 Jan 2010 01:41:03 +0100
> > > Subject: [PATCH] Handle UNC paths everywhere
> > >
> > > In Windows paths beginning with // are knows as UNC paths. They are
> > > absolute paths, usually referring to a shared resource on a server.
> >
> > And even a simple "cd" with them does not work.
> >
> > > Examples of legal UNC paths
> > >
> > > \\hub\repos\repo
> > > \\?\unc\hub\repos
> > > \\?\d:\repo
> > >
> > > Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> > > ---
> > > cache.h | 2 +-
> > > compat/basename.c | 2 +-
> > > compat/mingw.h | 8 +++++++-
> > > connect.c | 2 +-
> > > git-compat-util.h | 9 +++++++++
> > > path.c | 2 +-
> > > setup.c | 2 +-
> > > sha1_file.c | 20 ++++++++++++++++++++
> > > transport.c | 2 +-
> > > 9 files changed, 42 insertions(+), 7 deletions(-)
> >
> > Ouch. You should know better than to clutter non-Windows-specific parts
> > with that ugly kludge.
I did suspect that it would be better to handle unc paths differently than
the dos prefix, but I did not have the time to form arguments like Hannes
did.
So I think the first order of business is to add new code paths, rather
than modify existing ones. And then you can '#define
network_path_prefix_length(name) 0' in git-compat-util.h so that on
non-Windows platforms (i.e. our 1st class citizens), the code path can be
optimized out.
> > > diff --git a/cache.h b/cache.h
> > > index 767a50e..8f63640 100644
> > > --- a/cache.h
> > > +++ b/cache.h
> > > @@ -648,7 +648,7 @@ int safe_create_leading_directories_const(const char
> > > *path);
> > > char *enter_repo(char *path, int strict);
> > > static inline int is_absolute_path(const char *path)
> > > {
> > > - return path[0] == '/' || has_dos_drive_prefix(path);
> > > + return path[0] == '/' || has_win32_abs_prefix(path);
> >
> > Why? We can still keep the name. Well, maybe not, see below.
>
> I do think function names should imply something about their behaviour.
Actually, in this case, you do not even need to change anything, as Hannes
pointed out.
> > > diff --git a/compat/mingw.h b/compat/mingw.h
> > > index 1b528da..d1aa8be 100644
> > > --- a/compat/mingw.h
> > > +++ b/compat/mingw.h
> > > @@ -210,7 +210,13 @@ int winansi_fprintf(FILE *stream, const char
> > > *format, ...) __attribute__((format
> > > * git specific compatibility
> > > */
> > >
> > > -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] ==
> > > ':') +#define has_dos_drive_prefix(path) \
> > > + (isalpha(*(path)) && (path)[1] == ':')
> >
> > Why?
>
> To avoid very long lines and format this (now) set of related macros
> uniformely.
If you want your patch to go in (which you probably did not, you just
forgot to prefix the subject with RFC or RFH), you need it to be reviewed.
It is not a good idea to distract reviewers. Such a change does so.
> > > +#define has_unc_prefix(path) \
> > > + (is_dir_sep((path)[0]) && is_dir_sep((path)[1]))
> > > +#define has_win32_abs_prefix(path) \
> > > + (has_dos_drive_prefix(path) || has_unc_prefix(path))
> >
> > "c:hello.txt" is not an absolute path.
> Ok. Nevertheless that was how it was treated before, It's not relative,
> either, but some quasirelative thing. has_win32_quasi_abs_prefix?
No, none of this is good. You should not even pretend that the unc prefix
and the DOS drive prefix are the same. Just leave the old code paths
alone.
> > > diff --git a/git-compat-util.h b/git-compat-util.h
> > > index ef60803..0de9dac 100644
> > > --- a/git-compat-util.h
> > > +++ b/git-compat-util.h
> > > @@ -170,6 +170,15 @@ extern char *gitbasename(char *);
> > > #define has_dos_drive_prefix(path) 0
> > > #endif
> > >
> > > +#ifndef has_unc_prefix
> > > +#define has_unc_prefix(path) 0
> > > +#endif
> > > +
> > > +#ifndef has_win32_abs_prefix
> > > +#error no abs
> ouch, a leftover from trying to figure out a complation message.
Thought so.
> > In general, I am _very_ worried about your patch. It does not
> > acknowledge that there is a fundamental difference between DOS drive
> > prefixes and UNC paths, and not being able to "cd" to the latter is
> > just a symptom.
>
> As I said. Most programs including bash, but excluding cmd.exe can set
> the working directory to an UNC path. I cannot fix cmd.exe and rarely
> use it with git, but the patch helps even if you cannot cd from a UNC
> challenged shell.
The point I tried to make is that you should treat UNC paths differently,
and you can even leave the door open for non-Windows stuff. Just call the
function network_path_prefix_length() returning the length of the prefix.
If it becomes standard, say, on Linux, to have support for smb:// style
paths, we can always add support for that, too, and do not have to change
the name yet again.
Plus, by having separate code paths, you can be sure that you do not break
the existing ones.
And by defining network_path_prefix_length(path) to 0, the new code paths
can be optimized out on platforms which do not support network paths.
> > I am also not quite sure if you can get away with having the same
> > offset for both: if I have "C:\blah" and strip off "C:", I always have
> > a directory separator to bounce against, whereas I do not have that if
> > I strip off the two "\\" of a UNC path. Besides, I maintain that the
> > host name, and maybe even the share name, should not ever be stripped
> > off!
>
> When creating directoties you only strip them off for the purpose of
> finding paths to mkdir. The server and share part you cannot mkdir
> anyway, they must exist before attempting to create a directory, hence I
> skip past those portions.
I must have missed that. From what I saw, you treat the offset to be the
same as for DOS drive paths: 2 characters, which is definitely not enough.
Unfortunately, this patch needs more revisions, so it will probably not
make it into the upcoming Git for Windows, I am afraid. But then, we do
not need to let 3 months happen without a Git for Windows release.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Handle UNC paths everywhere
From: Johannes Schindelin @ 2010-01-26 11:01 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Johannes Sixt, Junio C Hamano
In-Reply-To: <201001252104.10328.robin.rosenberg@dewire.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 981 bytes --]
Hi,
On Mon, 25 Jan 2010, Robin Rosenberg wrote:
> måndagen den 25 januari 2010 18.34.01 skrev Johannes Schindelin:
> > I am also not quite sure if you can get away with having the same offset
> > for both: if I have "C:\blah" and strip off "C:", I always have a
> > directory separator to bounce against, whereas I do not have that if I
> > strip off the two "\\" of a UNC path. Besides, I maintain that the host
> > name, and maybe even the share name, should not ever be stripped off!
>
> Advices needed:
>
> d:somedir (when cwd=d:\msysgit, == /) may be tricky to fix.
This is a totally different issue from the UNC issue you brought up. May
I suggest to fix the UNC thing first, and if you feel inclined, take care
of the relative DOS drive path afterwards?
The relative DOS drive path is also not something I find overly pressing,
as I work from the Git bash, as you know, which does not translate
POSIX-style paths into such relative DOS drive paths.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] rebase -p: Preserve fast-forwardable merges
From: Johannes Schindelin @ 2010-01-26 11:14 UTC (permalink / raw)
To: Alex Scarborough; +Cc: git, Junio C Hamano, Michael Haggerty
In-Reply-To: <dd10f5481001251053x2ac9fa62r57a46f1231e36560@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1111 bytes --]
Hi,
On Mon, 25 Jan 2010, Alex Scarborough wrote:
> On Mon, Jan 25, 2010, Johannes Schindelin wrote:
> > On Fri, 22 Jan 2010, Alex Scarborough wrote:
> >
> > > Previously, rebase -p would not preserve a merge commit if the merge
> > > could be resolved as a fast-forward. rebase -p now passes --no-ff
> > > to git merge when recreating a merge commit, which ensures that
> > > merge commits created with git merge --no-ff are preserved.
> >
> > For my use case (well, it used to be my use case), namely keeping a
> > number of topic branches on top of an upstream up-to-date, this is not
> > the desired action. In my use case, merged topic branches should just
> > vanish, and not even leave a merge commit.
>
> I see. In that use case this patch would be rather irritating :)
>
> What do you think of adding a --no-ff option to git rebase which, when
> used with -p, will recreate merge commits even if they could be resolved
> as a fast-forward? That would leave your use case unchanged while
> giving my use case a way out, so to speak.
That would make most sense, I agree.
Thanks,
Dscho
^ permalink raw reply
* Re: Modern Git GUI
From: Michael J Gruber @ 2010-01-26 11:35 UTC (permalink / raw)
To: John Tapsell; +Cc: git, Johannes Schindelin, andre.harms, Matthieu Moy
In-Reply-To: <43d8ce651001250630p51761f6eyce8b89da6ffcd7b@mail.gmail.com>
[Re-adding cc culled by John. Bad boy :) ]
John Tapsell venit, vidit, dixit 25.01.2010 15:30:
> 2010/1/25 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> Hi,
>>
>> On Sun, 24 Jan 2010, Matthieu Moy wrote:
>>
>>> If you decide to go on with your project, either you want to carry it
>>> out alone, or you'll have to convince other developers to join. In the
>>> second case, a good starting point would be to explain why the other GUI
>>> are not good enough, and why you can't just contribute to them.
>>
>> Actually, a much better way would be to be positive, not negative. I.e.
>> to show something you did, which hopefully entices others to use your
>> project (and maybe contribute to it).
>
> Actually in this case, I don't think that would be good enough. My
> first thought would be "why didn't he just add this feature to an
> existing GUI rather than reinventing the wheel yet again". So there
> would really have to be good reasons why the other GUIs aren't a good
> enough starting point
A first step may be making the feature matrix in the wiki more honest -
I've been meaning to do this for ages... For example, a gui which simply
calls "gitk" for history view simply does not support history view
according to my books, at least not on the same level (gui
*integration*) as guis doing this natively. Or else gitk would have all
git-gui features and vice versa... or even plain git, for that matter.
Michael
^ permalink raw reply
* Re: [PATCH v4] Threaded grep
From: Fredrik Kuivinen @ 2010-01-26 11:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds, Johannes Sixt
In-Reply-To: <7vpr4x1x20.fsf@alter.siamese.dyndns.org>
On Mon, Jan 25, 2010 at 05:20:39PM -0800, Junio C Hamano wrote:
> Fredrik Kuivinen <frekui@gmail.com> writes:
>
> > The patch has been rebased on top of next.
> >
> > Additional changes since v3:
> >
> > * Fix some issues with Git's native pthreads implementation on
> > Windows (pthread_cond_broadcast is still used though).
> > * Fix style issues.
> > * When greping in a tree, allocate memory for the data buffer as
> > late as possible.
> > * Return void from grep_sha1_async and grep_file_async instead of
> > always returning 0.
>
> Thanks; I've fixed up a few old-style declaration header and queued the
> result to 'pu'.
I just noticed that I forgot to take the read_sha1 lock in
grep_tree. The result is a race condition in read_sha1_file.
Here is a patch to fix this. It applies on top of ae35c68 (Threaded
grep). Could you please squash it in? Let me know if you want me to
resend the entire patch instead.
(The only important thing is that we now take the read_sha1_mutex in
grep_tree. The rest is there just to make it self-consistent.)
- Fredrik
--- 8< ---
diff --git a/builtin-grep.c b/builtin-grep.c
index 7ecf222..6cc743d 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -75,10 +75,15 @@ static int todo_done;
static int all_work_added;
/* This lock protects all the variables above. */
-static pthread_mutex_t grep_lock;
+static pthread_mutex_t grep_mutex;
/* Used to serialize calls to read_sha1_file. */
-static pthread_mutex_t read_sha1_lock;
+static pthread_mutex_t read_sha1_mutex;
+
+#define grep_lock() pthread_mutex_lock(&grep_mutex)
+#define grep_unlock() pthread_mutex_unlock(&grep_mutex)
+#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
+#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
/* Signalled when a new work_item is added to todo. */
static pthread_cond_t cond_add;
@@ -93,10 +98,10 @@ static pthread_cond_t cond_result;
static void add_work(enum work_type type, char *name, void *id)
{
- pthread_mutex_lock(&grep_lock);
+ grep_lock();
while ((todo_end+1) % ARRAY_SIZE(todo) == todo_done) {
- pthread_cond_wait(&cond_write, &grep_lock);
+ pthread_cond_wait(&cond_write, &grep_mutex);
}
todo[todo_end].type = type;
@@ -107,16 +112,16 @@ static void add_work(enum work_type type, char *name, void *id)
todo_end = (todo_end + 1) % ARRAY_SIZE(todo);
pthread_cond_signal(&cond_add);
- pthread_mutex_unlock(&grep_lock);
+ grep_unlock();
}
static struct work_item *get_work(void)
{
struct work_item *ret;
- pthread_mutex_lock(&grep_lock);
+ grep_lock();
while (todo_start == todo_end && !all_work_added) {
- pthread_cond_wait(&cond_add, &grep_lock);
+ pthread_cond_wait(&cond_add, &grep_mutex);
}
if (todo_start == todo_end && all_work_added) {
@@ -125,7 +130,7 @@ static struct work_item *get_work(void)
ret = &todo[todo_start];
todo_start = (todo_start + 1) % ARRAY_SIZE(todo);
}
- pthread_mutex_unlock(&grep_lock);
+ grep_unlock();
return ret;
}
@@ -148,7 +153,7 @@ static void work_done(struct work_item *w)
{
int old_done;
- pthread_mutex_lock(&grep_lock);
+ grep_lock();
w->done = 1;
old_done = todo_done;
for(; todo[todo_done].done && todo_done != todo_start;
@@ -165,7 +170,7 @@ static void work_done(struct work_item *w)
if (all_work_added && todo_done == todo_end)
pthread_cond_signal(&cond_result);
- pthread_mutex_unlock(&grep_lock);
+ grep_unlock();
}
static void *run(void *arg)
@@ -181,11 +186,7 @@ static void *run(void *arg)
opt->output_priv = w;
if (w->type == WORK_SHA1) {
unsigned long sz;
- void* data;
-
- pthread_mutex_lock(&read_sha1_lock);
- data = load_sha1(w->identifier, &sz, w->name);
- pthread_mutex_unlock(&read_sha1_lock);
+ void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
hit |= grep_buffer(opt, w->name, data, sz);
@@ -218,8 +219,8 @@ static void start_threads(struct grep_opt *opt)
{
int i;
- pthread_mutex_init(&grep_lock, NULL);
- pthread_mutex_init(&read_sha1_lock, NULL);
+ pthread_mutex_init(&grep_mutex, NULL);
+ pthread_mutex_init(&read_sha1_mutex, NULL);
pthread_cond_init(&cond_add, NULL);
pthread_cond_init(&cond_write, NULL);
pthread_cond_init(&cond_result, NULL);
@@ -246,18 +247,18 @@ static int wait_all(void)
int hit = 0;
int i;
- pthread_mutex_lock(&grep_lock);
+ grep_lock();
all_work_added = 1;
/* Wait until all work is done. */
while (todo_done != todo_end)
- pthread_cond_wait(&cond_result, &grep_lock);
+ pthread_cond_wait(&cond_result, &grep_mutex);
/* Wake up all the consumer threads so they can see that there
* is no more work to do.
*/
pthread_cond_broadcast(&cond_add);
- pthread_mutex_unlock(&grep_lock);
+ grep_unlock();
for (i = 0; i < ARRAY_SIZE(threads); i++) {
void *h;
@@ -265,8 +266,8 @@ static int wait_all(void)
hit |= (int) (intptr_t) h;
}
- pthread_mutex_destroy(&grep_lock);
- pthread_mutex_destroy(&read_sha1_lock);
+ pthread_mutex_destroy(&grep_mutex);
+ pthread_mutex_destroy(&read_sha1_mutex);
pthread_cond_destroy(&cond_add);
pthread_cond_destroy(&cond_write);
pthread_cond_destroy(&cond_result);
@@ -274,6 +275,9 @@ static int wait_all(void)
return hit;
}
#else /* !NO_PTHREADS */
+#define read_sha1_lock()
+#define read_sha1_unlock()
+
static int wait_all(void)
{
return 0;
@@ -407,7 +411,11 @@ static void *load_sha1(const unsigned char *sha1, unsigned long *size,
const char *name)
{
enum object_type type;
- char *data = read_sha1_file(sha1, &type, size);
+ char *data;
+
+ read_sha1_lock();
+ data = read_sha1_file(sha1, &type, size);
+ read_sha1_unlock();
if (!data)
error("'%s': unable to read %s", name, sha1_to_hex(sha1));
@@ -596,7 +604,10 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
void *data;
unsigned long size;
+ read_sha1_lock();
data = read_sha1_file(entry.sha1, &type, &size);
+ read_sha1_unlock();
+
if (!data)
die("unable to read tree (%s)",
sha1_to_hex(entry.sha1));
^ permalink raw reply related
* Re: [PATCH v4] Threaded grep
From: Fredrik Kuivinen @ 2010-01-26 12:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001251542100.3574@localhost.localdomain>
On Tue, Jan 26, 2010 at 00:59, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> The profile for the threaded case says:
>
> 51.73% git libc-2.11.1.so [.] re_search_internal
> 11.47% git [kernel] [k] copy_user_generic_string
> 2.90% git libc-2.11.1.so [.] __strlen_sse2
> 2.66% git [kernel] [k] link_path_walk
> 2.55% git [kernel] [k] intel_pmu_enable_all
> 2.40% git [kernel] [k] __d_lookup
> 1.71% git libc-2.11.1.so [.] __GI___libc_malloc
> 1.55% git [kernel] [k] _raw_spin_lock
> 1.43% git [kernel] [k] sys_futex
> 1.30% git libc-2.11.1.so [.] __cfree
> 1.28% git [kernel] [k] intel_pmu_disable_all
> 1.25% git libc-2.11.1.so [.] __GI_memchr
> 1.14% git libc-2.11.1.so [.] _int_malloc
> 1.02% git [kernel] [k] effective_load
>
> and the only thing that makes me go "eh?" there is the strlen(). Why is
> that so hot? But locking doesn't seem to be the biggest issue, and in
> general I think this is all pretty good. The 'effective_load' thing is the
> scheduler, so there's certainly some context switching going on, probably
> still due to excessive synchronization, but it's equally clear that that
> is certainly not a dominant factor.
I see the strlen in my profiles as well, but I haven't figured out
where it comes from. I get the following:
51.16% git-grep /lib/tls/i686/cmov/libc-2.10.1.so
[.] 0x000000000b14c6
10.12% git-grep /lib/tls/i686/cmov/libc-2.10.1.so
[.] __GI_strlen
9.27% git-grep [kernel]
[k] __copy_to_user_ll
4.68% git-grep /lib/tls/i686/cmov/libc-2.10.1.so
[.] __memchr
1.72% git-grep [kernel]
[k] __d_lookup
1.18% git-grep /lib/i686/cmov/libcrypto.so.0.9.8
[.] sha1_block_asm_data_order
1.11% git-grep [kernel]
[k] __ticket_spin_lock
0.84% git-grep [vdso]
[.] 0x00000000b6c422
If I use perf record -g I get
10.39% git-grep /lib/tls/i686/cmov/libc-2.10.1.so
[.] __GI_strlen
|
|--99.05%-- look_ahead
| grep_buffer_1
| grep_buffer
| run
| start_thread
| __clone
|
|--0.64%-- grep_file
| grep_cache
| cmd_grep
| run_builtin
| handle_internal_command
| main
| __libc_start_main
| 0x804ae81
--0.32%-- [...]
This doesn't make much sense to me as look_ahead doesn't call strlen
(I compiled git with -O0 to avoid any issues with inlined functions).
But I haven't used perf so much, so maybe I'm reading the output the
wrong way.
> One potentially interesting data point is that if I make NR_THREADS be 16,
> performance goes down, and I get more locking overhead. So NR_THREADS of 8
> works well on this machine.
Interesting. I get the best results with 8 threads as well, but I only
have two cores.
> One worry is, of course, whether all regex() implementations are
> thread-safe. Maybe there are broken libraries that have hidden global
> state in them?
That would certainly be a problem. A quick google search didn't show
any known bugs. Of course, this doesn't tell us anything about the
unknown ones.
- Fredrik
^ permalink raw reply
* Re: For real now: bug tracking and secretary tasks in git
From: Jeff King @ 2010-01-26 12:26 UTC (permalink / raw)
To: Jan Krüger; +Cc: Git ML
In-Reply-To: <20100109013850.16f82412@perceptron>
[This reply is a bit late, but there doesn't seem to have been much
discussion, so...]
On Sat, Jan 09, 2010 at 01:38:50AM +0100, Jan Krüger wrote:
> Mass consensus in previous discussions[1][2] goes a bit like this:
>
> 1. It would be desirable to have people who do the work of interfacing
> between bug reporters and developers. These same people could make
> sure reports didn't get lost. These people are the *secretaries*.
> They should be pretty reliable.
>
> 2. People who contribute to git shouldn't be forced to work with the
> tracker. Having a tracker that isn't actively maintained by dedicated
> secretaries is pretty much worthless anyway, so there's no need to
> pretend that forcing developers to use a tracker interface is any
> kind of improvement.
>
> 3. The "human element" is important. For example, automatic reminders
> are a lot less valuable than reminders from an actual person.
I more or less agree with your approach, though I am a bit concerned
that because the process of moving information between the list and the
bug tracker is manual, bits of information will be lost unless the
secretaries are on their toes. Which means that people who submit bugs
will see no activity on their bug, even though it may have been
discussed on the list, and will consider the tracker to be crufty and
useless.
But that is not so much a criticism of your proposal, as a possible
thing that might go wrong. It's worth giving it a try and seeing what
happens.
I notice that there are not too many bugs in the tracker right now. If
this is going to be useful for ordinary users to submit bugs, it needs
to be publicized. JH suggested hosting it at kernel.org. I think that is
a reasonable idea, and certainly it needs a link from other git sites
(the wiki, and probably git-scm.org).
As far as the choice of flyspray, I'm not strongly against it, though I
suspect you would get more up-take from git developers (well, me,
anyway) if it was something that had a more git-ish interface. It would
be really nice to clone the bug db, edit, commit, and push bug updates.
Many of the distributed trackers support that. I recognize that for
ordinary users, we do still want some kind of web-based submitting and
bug reading interface. Surely somebody has built a git backend on one of
the existing trackers, or somebody has built a nice web interface on one
of the distributed trackers? It has been a while since I looked
seriously at this area, and I remember being a bit disappointed last
time.
And finally, thanks for starting a discussion on this issue. We talked a
little about it at the GitTogether, and what you proposed is in line
with what was said there, I think. My plan going forward from that
discussion (which I hadn't actually started implementing, though) was to
clean up my own personal todo list and start making it a bit more
public. The reason being that my list is not simply personal features,
but lots of bugs posted to the list that have not been dealt with, and
that I have either decided are probably actual bugs that need fixing, or
even ones that I have reproduced but need prodding to move forward on.
So sometime in the near future I'd like to clean up that list and dump
it into whatever sort of bug tracking we end up with.
-Peff
^ permalink raw reply
* Re: [PATCH 01/18] rebase -i: Make the condition for an "if" more transparent
From: Michael Haggerty @ 2010-01-26 12:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.1001251927460.8733@intel-tinevez-2-302>
Johannes Schindelin wrote:
> On Thu, 14 Jan 2010, Michael Haggerty wrote:
>
>> @@ -166,7 +166,8 @@ pick_one () {
>> parent_sha1=$(git rev-parse --verify $sha1^) ||
>> die "Could not get the parent of $sha1"
>> current_sha1=$(git rev-parse --verify HEAD)
>> - if test "$no_ff$current_sha1" = "$parent_sha1"; then
>> + if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1"
>
> Rather use &&, right?
Yes, that mistake was caused by my own ignorance about the portability
issues. This problem was already discussed [1] and the change has been
integrated into master.
Thanks,
Michael
[1] http://marc.info/?l=git&m=126344857120877&w=2
^ permalink raw reply
* Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
From: Jeff King @ 2010-01-26 13:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <1263980322-4142-2-git-send-email-gitster@pobox.com>
On Wed, Jan 20, 2010 at 01:38:41AM -0800, Junio C Hamano wrote:
> This adds a few more tests that exercises @{upstream} syntax by commands
> that operate differently when they are given branch name as opposed to a
> refname (i.e. where "master" and "refs/heads/master" makes a difference).
Overall this looks good, but there are a few minor defects. I haven't
had a chance to fix them yet, but here are tests showing them. I hope to
get to them pre-1.7.0, but please feel free to take a crack at them if
you want.
The first one is that @{usptream} silently becomes @{0}. I think
we need to double-check whether approxidate found absolutely nothing,
and complain if that is the case.
diff --git a/t/t0101-at-syntax.sh b/t/t0101-at-syntax.sh
new file mode 100755
index 0000000..da43386
--- /dev/null
+++ b/t/t0101-at-syntax.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='various @{whatever} syntax tests'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit one &&
+ test_commit two
+'
+
+check_at() {
+ echo "$2" >expect &&
+ git log -1 --format=%s "$1" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success '@{0} shows current' '
+ check_at @{0} two
+'
+
+test_expect_success '@{1} shows old' '
+ check_at @{1} one
+'
+
+test_expect_success '@{now} shows current' '
+ check_at @{now} two
+'
+
+test_expect_success '@{30.years.ago} shows old' '
+ check_at @{30.years.ago} one
+'
+
+test_expect_success 'silly approxidates work' '
+ check_at @{3.hot.dogs.and.30.years.ago} one
+'
+
+test_expect_failure 'complain about total nonsense' '
+ test_must_fail git log -1 --format=%s @{utter.bogosity}
+'
+
+test_done
The second one is that "log -g branch@{u}" shows the correct commits
(from the upstream of "branch"), but displays the incorrect reflog
information (it shows information for "branch", not for its upstream).
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 95c9b09..cbe1b25 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -107,4 +107,18 @@ test_expect_success 'checkout other@{u}' '
test_cmp expect actual
'
+cat >expect <<EOF
+commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
+Reflog: refs/heads/master@{0} (C O Mitter <committer@example.com>)
+Reflog message: branch: Created from HEAD
+Author: A U Thor <author@example.com>
+Date: Thu Apr 7 15:15:13 2005 -0700
+
+ 3
+EOF
+test_expect_failure 'log -g other@{u}' '
+ git log -1 -g other@{u} >actual &&
+ test_cmp expect actual
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH 01/18] rebase -i: Make the condition for an "if" more transparent
From: Johannes Schindelin @ 2010-01-26 13:11 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, gitster
In-Reply-To: <4B5EE257.8060204@alum.mit.edu>
Hi,
On Tue, 26 Jan 2010, Michael Haggerty wrote:
> Johannes Schindelin wrote:
> > On Thu, 14 Jan 2010, Michael Haggerty wrote:
> >
> >> @@ -166,7 +166,8 @@ pick_one () {
> >> parent_sha1=$(git rev-parse --verify $sha1^) ||
> >> die "Could not get the parent of $sha1"
> >> current_sha1=$(git rev-parse --verify HEAD)
> >> - if test "$no_ff$current_sha1" = "$parent_sha1"; then
> >> + if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1"
> >
> > Rather use &&, right?
>
> Yes, that mistake was caused by my own ignorance about the portability
> issues. This problem was already discussed [1] and the change has been
> integrated into master.
Thanks for the clarification, and sorry for not reviewing your series
before Junio applied it.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] git-cvsserver: allow regex metacharacters in CVSROOT
From: Gerrit Pape @ 2010-01-26 14:47 UTC (permalink / raw)
To: Junio C Hamano, git
When run in a repository with a path name containing regex metacharacters
(e.g. +), git-cvsserver failed to split the client request into CVSROOT
and module. Now metacharacters are disabled for the value of CVSROOT in
the perl regex so that directory names containing metacharacters are
handled properly.
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
git-cvsserver.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 2804106..13751db 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -408,7 +408,7 @@ sub req_Directory
$state->{localdir} = $data;
$state->{repository} = $repository;
$state->{path} = $repository;
- $state->{path} =~ s/^$state->{CVSROOT}\///;
+ $state->{path} =~ s/^\Q$state->{CVSROOT}\E\///;
$state->{module} = $1 if ($state->{path} =~ s/^(.*?)(\/|$)//);
$state->{path} .= "/" if ( $state->{path} =~ /\S/ );
--
1.6.6
^ permalink raw reply related
* [PATCH] builtin-config: Fix crash when using "-f <relative path>" from non-root dir
From: Johan Herland @ 2010-01-26 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Felipe Contreras
When your current directory is not at the root of the working tree, and you
use the "-f" option with a relative path, the current code segfaults since
argv has already been parsed, and argv[2] is now is a NULL pointer.
This patch replaces the incorrect argv[2] with the variable holding the
given config file name.
The bug was introduced by d64ec16... (git config: reorganize to use parseopt).
Signed-off-by: Johan Herland <johan@herland.net>
---
This is probably v1.7.0 material, and maybe even suitable for 'maint'.
AFAICS, d64ec16... has been with us since before v1.6.3-rc0.
...Johan
builtin-config.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-config.c b/builtin-config.c
index 2e3ef91..4bc46b1 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -358,7 +358,7 @@ int cmd_config(int argc, const char **argv, const char *unused_prefix)
if (!is_absolute_path(given_config_file) && prefix)
config_exclusive_filename = prefix_filename(prefix,
strlen(prefix),
- argv[2]);
+ given_config_file);
else
config_exclusive_filename = given_config_file;
}
--
1.6.6.rc2.5.g49666
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply related
* Re: [PATCH v4] Threaded grep
From: Linus Torvalds @ 2010-01-26 15:28 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <4c8ef71001260410l2afd2dbx17b6e216bd9e5d8@mail.gmail.com>
On Tue, 26 Jan 2010, Fredrik Kuivinen wrote:
>
> I see the strlen in my profiles as well, but I haven't figured out
> where it comes from.
Looks like this in gdb:
#0 0x0000003e3687f2e0 in __strlen_sse2 () from /lib64/libc.so.6
#1 0x0000003e368c04c5 in regexec@@GLIBC_2.3.4 () from /lib64/libc.so.6
#2 0x000000000047677a in look_ahead (opt=<value optimized out>,
name=<value optimized out>, buf=<value optimized out>,
size=<value optimized out>, collect_hits=<value optimized out>)
at grep.c:679
#3 grep_buffer_1 (opt=<value optimized out>, name=<value optimized out>,
buf=<value optimized out>, size=<value optimized out>,
collect_hits=<value optimized out>) at grep.c:790
so it's sadly internal to regex. It would be nice if there was a
non-string interface to regexec (ie a "buffer + length" instead of a
NUL-terminated string).
> If I use perf record -g I get
I suspect that libc isn't compiled with frame pointers, so call chains end
up being unreliable.
Linus
^ permalink raw reply
* [PATCH 00/12] Re: Makefile: add missing header dependency rules
From: Jonathan Nieder @ 2010-01-26 15:43 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <7v3a1u6bfz.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> These patches are independent of the jn/makefile topic. I sent them
>> before, but I screwed up the sender so nobody received them. Anyway,
>> perhaps they could be useful.
>
> I found all of them sensible but it seems to be based on version that is a
> bit behind...
Thanks. Patches 1-5 below are those I sent before, rebased against
maint. Since last time, remote-curl.o gained a few more dependencies,
which are included.
The remainder of this series is based on a merge of master and
patches 1-5:
Patch 6 adds another missing header dependency rule. It is not
included with the others because it applies to master, not maint.
Patches 7-12 revisit the topic of generating header dependency
rules on the fly. I updated them to produce patches 1-6, so I
thought, why not get some feedback on their present state?
Patch 7 clumps the header dependency rules together. I think this
makes them more readable; more importantly, later this will allow
ifdef'ing them out later in one fell swoop.
Patches 8-9 and 11 list all targets that can depend on headers in a
new OBJECTS variable, for use in patches 10 and 12.
Patch 10 generates header dependency rules on the fly. The resulting
dependency rules contain no useless dependencies (unlike the
hard-coded ones where almost everything depends on $(LIB_H)), so this
might be interesting to people wishing to speed up the
edit-compile-run cycle.
This only works with gcc. It should be possible to teach the relevant
options to clink.pl if there is demand, but not SCO cc.
Patch 12 is what I am most interested in: it uses the computed
dependencies to check that the hard-coded dependency rules are
correct. In the long term, maybe we will be able to get rid of the
hard-coded header dep rules altogether; in the short term, I am hoping
this checker can be useful for avoiding problems with the hard-coded
rules without spending too much human time on it.
Enjoy,
Jonathan Nieder (12):
Makefile: add missing header file dependencies
Makefile: make sure test helpers are rebuilt when headers change
Makefile: remove wt-status.h from LIB_H
Makefile: clean up http-walker.o dependency rules
Makefile: drop dependency on $(wildcard */*.h)
Makefile: transport.o depends on branch.h now
Makefile: rearrange dependency rules
Makefile: disable default implicit rules
Makefile: list generated object files in OBJECTS macro
Makefile: lazily compute header dependencies
Makefile: list standalone program object files in PROGRAM_OBJS
Teach Makefile to check header dependencies
^ permalink raw reply
* [PATCH 01/12] Makefile: add missing header file dependencies
From: Jonathan Nieder @ 2010-01-26 15:44 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
LIB_H is missing exec_cmd.h and color.h. cache.h includes
SHA1_HEADER, and thus so does almost everything else, so add that
to LIB_H, too. xdiff-interface.h is not included by any header
files, but so many source files use xdiff that it is simplest to
include it in LIB_H, too.
xdiff-interface.o uses the xdiff library heavily; let it depend
on all xdiff headers to avoid needing to keep track of which
headers it uses.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index fd7f51e..bad8f80 100644
--- a/Makefile
+++ b/Makefile
@@ -447,6 +447,7 @@ LIB_H += blob.h
LIB_H += builtin.h
LIB_H += cache.h
LIB_H += cache-tree.h
+LIB_H += color.h
LIB_H += commit.h
LIB_H += compat/bswap.h
LIB_H += compat/cygwin.h
@@ -457,6 +458,7 @@ LIB_H += delta.h
LIB_H += diffcore.h
LIB_H += diff.h
LIB_H += dir.h
+LIB_H += exec_cmd.h
LIB_H += fsck.h
LIB_H += git-compat-util.h
LIB_H += graph.h
@@ -499,6 +501,8 @@ LIB_H += unpack-trees.h
LIB_H += userdiff.h
LIB_H += utf8.h
LIB_H += wt-status.h
+LIB_H += xdiff-interface.h
+LIB_H += xdiff/xdiff.h
LIB_OBJS += abspath.o
LIB_OBJS += advice.o
@@ -1281,10 +1285,12 @@ endif
ifdef BLK_SHA1
SHA1_HEADER = "block-sha1/sha1.h"
LIB_OBJS += block-sha1/sha1.o
+ LIB_H += block-sha1/sha1.h
else
ifdef PPC_SHA1
SHA1_HEADER = "ppc/sha1.h"
LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
+ LIB_H += ppc/sha1.h
else
SHA1_HEADER = <openssl/sha.h>
EXTLIBS += $(LIB_4_CRYPTO)
@@ -1620,9 +1626,9 @@ 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 http-push.o remote-curl.o: http.h
-http.o http-walker.o: $(LIB_H)
+http.o http-walker.o remote-curl.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,$^) \
@@ -1637,14 +1643,25 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
+builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
+builtin-bundle.o bundle.o transport.o: bundle.h
+builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
+builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
+builtin-send-pack.o transport.o: send-pack.h
+builtin-log.o builtin-shortlog.o: shortlog.h
+builtin-prune.o builtin-reflog.o reachable.o: reachable.h
builtin-revert.o wt-status.o: wt-status.h
+builtin-tar-tree.o archive-tar.o: tar.h
+builtin-pack-objects.o: thread-utils.h
+http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.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-interface.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)
--
1.6.6
^ permalink raw reply related
* [PATCH 02/12] Makefile: make sure test helpers are rebuilt when headers change
From: Jonathan Nieder @ 2010-01-26 15:45 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
It is not worth the bother to maintain an up-to-date list of
which headers each test helper uses, so depend on $(LIB_H) to
catch them all.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This applies to maint. Please let me know if you are interested
in the corresponding patch for master.
Makefile | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/Makefile b/Makefile
index bad8f80..1670ee8 100644
--- a/Makefile
+++ b/Makefile
@@ -402,6 +402,18 @@ PROGRAMS += git-upload-pack$X
PROGRAMS += git-var$X
PROGRAMS += git-http-backend$X
+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
+
# List built-in command $C whose implementation cmd_$C() is not in
# builtin-$C.o but is linked in as part of some other command.
BUILT_INS += $(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS))
@@ -695,6 +707,8 @@ BUILTIN_OBJS += builtin-verify-pack.o
BUILTIN_OBJS += builtin-verify-tag.o
BUILTIN_OBJS += builtin-write-tree.o
+TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
+
GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
EXTLIBS =
@@ -1642,7 +1656,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
+$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h)
builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
builtin-bundle.o bundle.o transport.o: bundle.h
builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
@@ -1732,18 +1746,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.
@@ -1763,9 +1765,7 @@ test-delta$X: diff-delta.o patch-delta.o
test-parse-options$X: parse-options.o
-test-parse-options.o: parse-options.h
-
-.PRECIOUS: $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
+.PRECIOUS: $(TEST_OBJS)
test-%$X: test-%.o $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
--
1.6.6
^ permalink raw reply related
* [PATCH 03/12] Makefile: remove wt-status.h from LIB_H
From: Jonathan Nieder @ 2010-01-26 15:46 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
A list of the few translation units using this header is
half-populated already. Including the dependency on this header
twice (once explicitly, once through LIB_H) makes it difficult to
figure out where future headers should be added to the Makefile.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 1670ee8..8cf6383 100644
--- a/Makefile
+++ b/Makefile
@@ -512,7 +512,6 @@ LIB_H += tree-walk.h
LIB_H += unpack-trees.h
LIB_H += userdiff.h
LIB_H += utf8.h
-LIB_H += wt-status.h
LIB_H += xdiff-interface.h
LIB_H += xdiff/xdiff.h
@@ -1664,7 +1663,7 @@ builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
builtin-send-pack.o transport.o: send-pack.h
builtin-log.o builtin-shortlog.o: shortlog.h
builtin-prune.o builtin-reflog.o reachable.o: reachable.h
-builtin-revert.o wt-status.o: wt-status.h
+builtin-commit.o builtin-revert.o wt-status.o: wt-status.h
builtin-tar-tree.o archive-tar.o: tar.h
builtin-pack-objects.o: thread-utils.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
--
1.6.6
^ permalink raw reply related
* [PATCH 04/12] Makefile: clean up http-walker.o dependency rules
From: Jonathan Nieder @ 2010-01-26 15:46 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
http-walker.o depends on http.h twice: once in the rule listing
files that use http.h, and again in the rule explaining how to
build it. Messy.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 8cf6383..593801a 100644
--- a/Makefile
+++ b/Makefile
@@ -1628,7 +1628,7 @@ http.o: http.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $<
ifdef NO_EXPAT
-http-walker.o: http-walker.c http.h GIT-CFLAGS
+http-walker.o: http-walker.c GIT-CFLAGS
$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DNO_EXPAT $<
endif
--
1.6.6
^ permalink raw reply related
* [PATCH 05/12] Makefile: drop dependency on $(wildcard */*.h)
From: Jonathan Nieder @ 2010-01-26 15:46 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>
The files this pulls in are already pulled in by other dependency
rules (some recently added).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 593801a..98810b2 100644
--- a/Makefile
+++ b/Makefile
@@ -1655,7 +1655,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h)
+$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
builtin-bundle.o bundle.o transport.o: bundle.h
builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
--
1.6.6
^ 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