* Re: [PULL] svn-fe updates for master or next
From: Jonathan Nieder @ 2012-01-27 7:20 UTC (permalink / raw)
To: David Barr
Cc: Junio C Hamano, Scott Chacon, Brian Gernhardt, david,
Ramkumar Ramachandra, git, Dmitry Ivankov
In-Reply-To: <CAFfmPPN-g+Lk2n9tzXe=CfyK8OZ7nGu4NwX0cXjtxS0W7SwPHA@mail.gmail.com>
David Barr wrote:
> I do think we need to gather Dmitry's work on svn-fe
Listed at [1]. Thanks for the nice and well maintained overview,
Dmitry.
> and propose a
> front-end for core so that it is no longer relegated to contrib.
Yep. Should such a remote helper link directly to the vcs-svn lib, or
would it make sense to start with a script that makes use of svn-fe
(possibly with a different name once it is hoisted out of contrib)?
[1] http://divanorama.github.com/gsoc11/streams.html
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Johannes Sixt @ 2012-01-27 7:27 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120127054216.GA23633@sigill.intra.peff.net>
Am 1/27/2012 6:42, schrieb Jeff King:
> That being said, I think it would be nicer for projects to carry meta
> information like this out-of-tree in a special ref. It's just simpler to
> work with, and it means the project's source isn't polluted with extra
> junk.
Really? I doubt that carrying configuration in a special ref outside the
normal contents will have any practical relevance:
To manage such a config file would mean to switch to a branch with
entirely different contents. But before you can test the new configuration
in action, you have to commit, switch branches, which exchanges the
worktree completely; and if the config change didn't work out, repeat the
process (and if we are talking about source code repository, this usally
includes a complete rebuild). Sure, you could keep the config branch in a
separate repository, but, again, how do you test an updated configuration?
It is not funny, and nobody will go this route.
Which raises doubts about the usefulness of the include.ref feature.
-- Hannes
^ permalink raw reply
* Re: git rebase likes to fail miserably on Mac OS X Lion
From: Stefan Haller @ 2012-01-27 7:59 UTC (permalink / raw)
To: Joshua Jensen, git@vger.kernel.org
In-Reply-To: <4F223543.3080903@workspacewhiz.com>
Joshua Jensen <jjensen@workspacewhiz.com> wrote:
> On 4 different Mac OS X Lion machines, rebasing my commits (I currently
> have 14 of them) yields either of the following _consistently_ across
> varied repositories:
As far as I can tell, these are two completely unrelated problems.
> fatal: Unable to create
> '/Users/joshua/src/project/.git/index.lock': File exists.
In my experience, this is caused by Xcode 4's builtin git support. It
happens when you have your project open in Xcode 4 while you try to
rebase it. It doesn't seem to be possible to turn off Xcode's git
support (I never use it), so the only workaround seems to be to close
the Xcode project before you rebase.
> error: Your local changes to the following files would be
> overwritten by merge:
As a workaround for this one, try
git config --global core.trustctime false
This helps for me, but I have no idea what the implications are or why
it would be necessary.
(Out of town until Feb 5; don't expect timely replies.)
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-27 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Jonathan Nieder, git
In-Reply-To: <7vr4ym2rad.fsf@alter.siamese.dyndns.org>
On Thu, Jan 26, 2012 at 8:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> No thanks. IMHO, this is already too much code for too little gain.
>
> Thanks for bringing a bit of sanity. You have already said it "In which
> way is git different from other tools that execvp other programs?" earlier
> (http://thread.gmane.org/gmane.comp.version-control.git/171755/focus=171848)
Well, one tool that it differs from is bash (although bash uses execve
directly I think). Personally I think this whole thing essentially a
lack of information from execv*. Also, I do agree that the code
required for this is quite more than I would have liked, but it will
reduce confusion when things go wrong. It's when things go wrong that
people get annoyed. Annoyed people look for greener grass. If that bit
of annoyance could be reduced, why not go the extra mile for that
little bit of gain?
Being as it is, I'll stop working on this. If this was pretty much
going to be /dev/null'ed from the beginning, I'd rather have heard it
after my first patches.
In any case, it has been an education so far. Thanks for that. And if
there's any issue you think I could start tackling, please don't
hesitate to cc me.
Cheers,
Frans
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Jonathan Nieder @ 2012-01-27 8:48 UTC (permalink / raw)
To: Frans Klaver; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <CAH6sp9NEnkDY-BCccW9VM3waxg8sG8zV5-rVAuMUfZ9rji4-Qw@mail.gmail.com>
(+cc: Jeff because mentioning a pagination side-issue [*])
Frans Klaver wrote:
> If this was pretty much
> going to be /dev/null'ed from the beginning, I'd rather have heard it
> after my first patches.
Almost always when a developer has an itch, it is _possible_ to
massage a patch that scratches it into something acceptable to others.
And whether it is worth the trouble in terms of time is something that
only that developer can decide.
So no, I would not say these patches were not doomed from the
beginning. However, I certainly agree that in their current form they
are more complicated than the use case justifies.
There is a tension between requirements that leaves me oddly
uncomfortable with the series:
a. on one hand, it would be nice to preserve all the current features
of execvp(), which makes the approach of only doing post-mortem
analysis after a failed execvp appealing;
b. on the other hand, it would be nice [*] to avoid launching a pager
only in order to call execvp for a command that does not exist when
the fallback might be to an alias to a command that does not want a
pager. That would require figuring out in advance that execvp
would fail with ENOENT and missing out on possible system extensions
that allow execvp to run shell built-in commands not existing on
the filesystem.
I want to like (b), but the downside seems unacceptable. I honestly
don't know if something like (a) would be a good idea if well
executed, so I was happy to have the opportunity to try to help
massage these patches into a form that would make the answer more
obvious.
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-27 9:11 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <20120127084845.GC806@burratino>
On Fri, Jan 27, 2012 at 9:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> If this was pretty much
>> going to be /dev/null'ed from the beginning, I'd rather have heard it
>> after my first patches.
>
> Almost always when a developer has an itch, it is _possible_ to
> massage a patch that scratches it into something acceptable to others.
> And whether it is worth the trouble in terms of time is something that
> only that developer can decide.
Hannes' reaction and Junio's to his didn't give me the impression they
even saw a possibility.
> So no, I would not say these patches were not doomed from the
> beginning. However, I certainly agree that in their current form they
> are more complicated than the use case justifies.
Good. That's something we can work on.
> There is a tension between requirements that leaves me oddly
> uncomfortable with the series:
>
> a. on one hand, it would be nice to preserve all the current features
> of execvp(), which makes the approach of only doing post-mortem
> analysis after a failed execvp appealing;
>
> b. on the other hand, it would be nice [*] to avoid launching a pager
> only in order to call execvp for a command that does not exist when
> the fallback might be to an alias to a command that does not want a
> pager. That would require figuring out in advance that execvp
> would fail with ENOENT and missing out on possible system extensions
> that allow execvp to run shell built-in commands not existing on
> the filesystem.
Just for my understanding: before a command is executed, a pager
(less/more or so) is started? We want to avoid starting the pager if
we won't be able to execute the command?
> I want to like (b), but the downside seems unacceptable.
The downside being: having to figure out what execvp is going to do?
That would be tantamount to writing your own execvp.
> I honestly
> don't know if something like (a) would be a good idea if well
> executed, so I was happy to have the opportunity to try to help
> massage these patches into a form that would make the answer more
> obvious.
Given the above information, I'm happy to work on this to see if we
can mould it into something usable. Since the impact seems to go
beyond figuring out why execvp failed, I'm probably going to need some
help.
For now, I'll go through your suggestions and see what it produces.
We'll go from there.
Thanks for the heads-up.
^ permalink raw reply
* Re: Test t9500 fails if Time::HiRes is missing
From: Jakub Narębski @ 2012-01-27 9:18 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Hallvard Breien Furuseth, git
In-Reply-To: <CACBZZX4cjcY5d3mPJAV+rbSTqCEUOrF=_dd3ny_jSM++G-Bg1Q@mail.gmail.com>
On Mon, Jan 23, 2012 at 10:42 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Jan 23, 2012 at 05:50, Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> wrote:
>>
>> t9500-gitweb-standalone-no-errors fails: Git 1.7.9.rc2/1.7.8.4, RHEL
>> 6.2, Perl 5.10.1. Reverting 3962f1d756ab41c1d180e35483d1c8dffe51e0d1
>> fixes it. The commit expects Time::HiRes to be present, but RedHat
>> has split it out to a separate RPM perl-Time-HiRes. Better add a
>> comment about that, so it doesn't get re-reverted.
>>
>> Or pacify the test and expect gitweb@RHEL-users to install the RPM:
>>
>> --- git-1.7.9.rc2/t/gitweb-lib.sh~
>> +++ git-1.7.9.rc2/t/gitweb-lib.sh
>> @@ -113,4 +113,9 @@
>> test_done
>> }
>>
>> +perl -MTime::HiRes -e 0 >/dev/null 2>&1 || {
>> + skip_all='skipping gitweb tests, Time::HiRes module not available'
>> + test_done
>> +}
>> +
>> gitweb_init
>
> [Adding Jakub to CC]
>
> This doesn't actually fix the issue, it only sweeps it under the rug
> by making the tests pass, gitweb will still fail to compile on Red
> Hat once installed.
>
> I think the right solution is to partially revert
> 3962f1d756ab41c1d180e35483d1c8dffe51e0d1, but add a comment in the
> code indicating that it's to deal with RedHat's broken fork of Perl.
>
> However even if it's required in an eval it might still fail at
> runtime in the reset_timer() function, which'll need to deal with it
> too.
I'll try to send a fix today. Time::HiRes is needed only for optional timing
info.
--
Jakub Narebski
^ permalink raw reply
* Re: Test t9500 fails if Time::HiRes is missing
From: Ævar Arnfjörð Bjarmason @ 2012-01-27 9:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Hallvard Breien Furuseth, git, Jakub Narebski
In-Reply-To: <7v8vkt1yry.fsf@alter.siamese.dyndns.org>
On Fri, Jan 27, 2012 at 06:48, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> This doesn't actually fix the issue, it only sweeps it under the rug
>> by making the tests pass, gitweb will still fail to compile on Red
>> Hat once installed.
>
> In the short term for 1.7.9, let's at least warn users about this issue.
>
> -- >8 --
> Subject: INSTALL: warn about recent Fedora breakage
>
> Recent releases of Redhat/Fedora are reported to ship Perl binary package
> with some core modules stripped away (see http://lwn.net/Articles/477234/)
> against the upstream Perl5 people's wishes. The Time::HiRes module used by
> gitweb one of them.
Since I wrote that E-Mail I learned what RedHat was doing, I think
that's a far better option. They're splitting up the perl core into
multiple packages, and anyone who has issues with this on RedHat can
trivially just install those packages. So we should just note it in
the INSTALL file as a platform-specific issue and leave it at that.
We *could* deal with this in our code, but I don't think dealing with
every vendor's slightly different perl version is a viable strategy in
the long run.
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Ævar Arnfjörð Bjarmason @ 2012-01-27 9:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120127003241.GA15165@sigill.intra.peff.net>
On Fri, Jan 27, 2012 at 01:32, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 27, 2012 at 01:02:52AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Jan 26, 2012 at 08:37, Jeff King <peff@peff.net> wrote:
>> > This patch introduces an include directive for config files.
>> > It looks like:
>> >
>> > [include]
>> > path = /path/to/file
>>
>> Very nice, I'd been meaning to resurrect my gitconfig.d series, and
>> this series implements a lot of the structural changes needed for that
>> sort of thing.
>
> Yeah, that seems like a reasonable thing to do. It could make life
> easier for package managers (I think the only reason it has not come up
> much is that there simply isn't a lot of third-party git config).
>
>> What do you think of an option (e.g. include.gitconfig_d = true) that
>> would cause git to look in:
>>
>> /etc/gitconfig.d/*
>> ~/.gitconfig.d/*
>> .git/config.d/*
>
> Hmm. Is that really worth having an option? I.e., why not just always
> check those directories?
You're right, always just including those directories is a much better
option, an extra stat() doesn't cost us much.
Thanks again for working on this.
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Jonathan Nieder @ 2012-01-27 9:41 UTC (permalink / raw)
To: Frans Klaver; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <CAH6sp9O7P8bmYA66fY754mn=ogp8OP1i3KQuE_hnrTY46nNAxw@mail.gmail.com>
Frans Klaver wrote:
> Just for my understanding: before a command is executed, a pager
> (less/more or so) is started? We want to avoid starting the pager if
> we won't be able to execute the command?
See [1] for an example of a recent patch touching the relevant
code path.
For example: if I run "git --paginate foo", foo is an alias for bar,
and the "[pager] bar" configuration is set to point to "otherpager",
then without this safety git launches the default pager in preparation
for running git-foo, receives ENOENT from execvp("git-foo"), and then
the pager has already been launched and it is too late to launch
otherpager instead.
> On Fri, Jan 27, 2012 at 9:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> I want to like (b), but the downside seems unacceptable.
>
> The downside being: having to figure out what execvp is going to do?
> That would be tantamount to writing your own execvp.
Exactly.
>> I honestly
>> don't know if something like (a) would be a good idea if well
>> executed, so I was happy to have the opportunity to try to help
>> massage these patches into a form that would make the answer more
>> obvious.
>
> Given the above information, I'm happy to work on this
I see.
Well, as I said, I don't know. :) And I don't want to give false
hopes --- it's perfectly possible and not even unlikely that this is a
dead end and any patch in this direction will turn out not to be a
good idea and not applied.
That's part of why I was really grateful to Hannes for the reminder to
take a step back for a moment and consider whether it's worth it.
Maybe there's another way or a more targetted way to take care of the
motivational original confusing scenario that leads to execvp errors.
(By the way, can you remind me which one that was?)
Jonathan
[1] http://thread.gmane.org/gmane.comp.version-control.git/179635
^ permalink raw reply
* Re: [RFC/PATCH 0/4] config include directives
From: Ævar Arnfjörð Bjarmason @ 2012-01-27 9:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120126073547.GA28689@sigill.intra.peff.net>
On Thu, Jan 26, 2012 at 08:35, Jeff King <peff@peff.net> wrote:
> This series provides a way for config files to include other config
> files in two ways:
>
> 1. From other files in the filesystem. This is implemented by patch 1
> below, and is hopefully straightforward and uncontroversial. See
> that patch for more rationale.
>
> 2. From blobs in the repo. This is implemented by patch 4, with
> patches 2 and 3 providing the necessary refactoring. This
> is one way of implementing the often asked-for "respect shared
> config inside the repo" feature, but attempts to mitigate some of
> the security concerns. The interface for using it safely is a bit
> raw, but I think it's a sane building block, and somebody could
> write a fancier shared-config updater on top of it if they wanted
> to.
>
> [1/4]: config: add include directive
> [2/4]: config: factor out config file stack management
> [3/4]: config: support parsing config data from buffers
> [4/4]: config: allow including config from repository blobs
I expect you've thought about this, but our current API is (from
add.c):
git_config(add_config, NULL);
Followed by:
static int add_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "add.ignoreerrors") ||
!strcmp(var, "add.ignore-errors")) {
ignore_add_errors = git_config_bool(var, value);
return 0;
}
return git_default_config(var, value, cb);
}
I.e. that function gets called with one key at a time, and stashes it
to a local value.
If you write the function like that it means your patch series just
works since values encountered later will override earlier ones, but
have you checked git's code to make sure we don't have anything like:
static int ignore_add_errors_is_set = 0;
static int add_config(const char *var, const char *value, void *cb)
{
if (!ignore_add_errors_is_set &&
(!strcmp(var, "add.ignoreerrors") ||
!strcmp(var, "add.ignore-errors"))) {
ignore_add_errors = git_config_bool(var, value);
ignore_add_errors_is_set = 1;
return 0;
}
return git_default_config(var, value, cb);
}
Which would mean that the include config support would be silently
ignored.
^ permalink raw reply
* Re: make install rewrites source files
From: Hallvard B Furuseth @ 2012-01-27 9:46 UTC (permalink / raw)
To: Phillip Susi; +Cc: git
In-Reply-To: <4F1DC2F7.2070502@ubuntu.com>
On Mon, 23 Jan 2012 15:28:39 -0500, Phillip Susi <psusi@ubuntu.com>
wrote:
> On 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:
>> However, make install should not write to the source directory in
>> any case. That fails as root if root lacks write access there, due
>> to NFS mounts that map root to nobody etc. At least git-instaweb
>> and GIT-BUILD-OPTIONS are rewritten. You can simulate this with su
>> nobody -s /bin/bash -c 'make -k install' after configuring with
>> prefix=<directory owned by nobody>.
>
> If you want to build locally from a read only nfs mount, then you
> should run the configure script in a local directory:
>
> mkdir /tmp/build
> (...)
Not a read-only nfs mount. Just an ordinary remote mount where root
on the local host is mapped to nobody on the remote host. (Having
local root access does not mean you should get root on the remote.)
In any case, it's normal practice to do as little as possible as root,
and also to at least try not write to the source dir during install.
BTW, building in /tmp can be nasty to other users when you don't know
how much space the build (and maybe test) will use, so you may need
access to some other local dir.
--
Hallvard
^ permalink raw reply
* Re: Test t9500 fails if Time::HiRes is missing
From: Hallvard B Furuseth @ 2012-01-27 10:15 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Jakub Narebski
In-Reply-To: <CACBZZX4cjcY5d3mPJAV+rbSTqCEUOrF=_dd3ny_jSM++G-Bg1Q@mail.gmail.com>
On Mon, 23 Jan 2012 10:42:02 +0100, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Jan 23, 2012 at 05:50, Hallvard Breien Furuseth
> <h.b.furuseth@usit.uio.no> wrote:
>> Or pacify the test and expect gitweb@RHEL-users to install the RPM:
>>
>> --- git-1.7.9.rc2/t/gitweb-lib.sh~
>> +++ git-1.7.9.rc2/t/gitweb-lib.sh
>> @@ -113,4 +113,9 @@
>> test_done
>> }
>>
>> +perl -MTime::HiRes -e 0 >/dev/null 2>&1 || {
>> + skip_all='skipping gitweb tests, Time::HiRes module not
>> available'
>> + test_done
>> +}
>> +
>> gitweb_init
>
> [Adding Jakub to CC]
>
> This doesn't actually fix the issue, it only sweeps it under the rug
> by making the tests pass, gitweb will still fail to compile on Red
> Hat once installed.
Is that relevant? gitweb-lib.sh already has code to pass the tests if
Encode, CGI, CGI::Util or CGI::Carp are missing. I just added another.
--
Hallvard
^ permalink raw reply
* Re: Test t9500 fails if Time::HiRes is missing
From: Jakub Narębski @ 2012-01-27 10:59 UTC (permalink / raw)
To: Hallvard B Furuseth; +Cc: Ævar Arnfjörð Bjarmason, git
In-Reply-To: <69c90e626682e60d33bebcc6d3ff3fdb@ulrik.uio.no>
On Fri, Jan 27, 2012 at 11:15 AM, Hallvard B Furuseth
<h.b.furuseth@usit.uio.no> wrote:
> On Mon, 23 Jan 2012 10:42:02 +0100, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Mon, Jan 23, 2012 at 05:50, Hallvard Breien Furuseth
>> <h.b.furuseth@usit.uio.no> wrote:
>>>
>>> Or pacify the test and expect gitweb@RHEL-users to install the RPM:
>>>
>>> --- git-1.7.9.rc2/t/gitweb-lib.sh~
>>> +++ git-1.7.9.rc2/t/gitweb-lib.sh
>>> @@ -113,4 +113,9 @@
>>> test_done
>>> }
>>>
>>> +perl -MTime::HiRes -e 0 >/dev/null 2>&1 || {
>>> + skip_all='skipping gitweb tests, Time::HiRes module not available'
>>> + test_done
>>> +}
>>> +
>>> gitweb_init
>>
>>
>> [Adding Jakub to CC]
>>
>> This doesn't actually fix the issue, it only sweeps it under the rug
>> by making the tests pass, gitweb will still fail to compile on Red
>> Hat once installed.
>
>
> Is that relevant? gitweb-lib.sh already has code to pass the tests if
> Encode, CGI, CGI::Util or CGI::Carp are missing. I just added another.
The difference is that:
1.) Time::HiRes is a core Perl module, so theoretically it should be always
installed.
2.) Time::HiRes is not really required for gitweb to work, only for optional
feature (timing information).
--
Jakub Narebski
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-27 11:46 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <20120127094145.GA2611@burratino>
On Fri, Jan 27, 2012 at 10:41 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Frans Klaver wrote:
>
>> Just for my understanding: before a command is executed, a pager
>> (less/more or so) is started? We want to avoid starting the pager if
>> we won't be able to execute the command?
>
> See [1] for an example of a recent patch touching the relevant
> code path.
I'll have a look at that.
> For example: if I run "git --paginate foo", foo is an alias for bar,
> and the "[pager] bar" configuration is set to point to "otherpager",
> then without this safety git launches the default pager in preparation
> for running git-foo, receives ENOENT from execvp("git-foo"), and then
> the pager has already been launched and it is too late to launch
> otherpager instead.
Something to be looked into then.
> Well, as I said, I don't know. :) And I don't want to give false
> hopes --- it's perfectly possible and not even unlikely that this is a
> dead end and any patch in this direction will turn out not to be a
> good idea and not applied.
Hm don't worry about false hopes. I don't insist on having some of my
work actually in if there's no point in including it. Contributing to
the research is good enough for me if we can come to a conclusion that
we can present to people running into similar issues.
> That's part of why I was really grateful to Hannes for the reminder to
> take a step back for a moment and consider whether it's worth it.
> Maybe there's another way or a more targetted way to take care of the
> motivational original confusing scenario that leads to execvp errors.
I wonder.
> (By the way, can you remind me which one that was?)
I'll even summarize my thinking and motivation about this.
I was executing the test suite on my PC. Some test for aliases failed
-- git said EACCES, while for aliases one would expect ENOENT. For
users expecting an alias to be executed, "cannot execute git-frotz:
Access Denied" will be rather confusing. "Huh? Access denied? The file
doesn't even exist?!". It took me quite some debugging in git to track
this down to an inaccessible PATH entry. As someone who didn't know
anything of the git internal code it took quite a bit of learning as
well just to find out where we'd end up in the first place. It
bothered me (and still does) that execve uses EACCES for at least four
different errors:
...
EACCES Search permission is denied on a component of the path
prefix of filename or the
name of a script interpreter. (See also path_resolution(7).)
EACCES The file or a script interpreter is not a regular file.
EACCES Execute permission is denied for the file or a script or
ELF interpreter.
EACCES The file system is mounted noexec.
...
Anyway, reading through that man page later on I found that a lot of
errors are only mentioned once, but do contain 'or' in the problem
description, like the first one of the EACCES items. ENOENT does that
as well:
ENOENT The file filename or a script or ELF interpreter does not
exist, or a shared
library needed for file or interpreter cannot be found.
I then additionally figured that always silently passing ENOENT is a
bad thing to do, because it simply can mean more than just "The file
you asked for cannot be found". It means "something required cannot be
found". My resulting view on this is basically that the execvp error
handling git currently has, is lacking a nuance that is necessary for
effective debugging. As I said earlier, it's when things go wrong
people get annoyed. Even more so if you don't provide them with
pointers to what might be wrong.
It also bothers me that to effectively debug program execution errors,
you have to know that git uses execvp, and you have to know how it
behaves. I would label that "implementation details" and as a user I
really don't want to be bothered by that, especially not as a new
user. For that reason I would have liked to end up with something like
bash does. I would rather see "Hey dummy, can't access /some/path" or
"Hey dummy, you ask for an interpreter that I have no acces to" than
"Well we got EACCES, so check the following things: Do we have search
permission on.... Is it a regular file? mounted noexec?..." or "We got
EACCES, check man execve(2) for possible reasons", although I'd agree
that even that would be slightly better than "We got EACCES".
So take of your git-guru hat and put on your new-git-user one and let
it simmer for a while.
^ permalink raw reply
* Re: make install rewrites source files
From: Hallvard Breien Furuseth @ 2012-01-27 13:11 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Junio C Hamano, git
In-Reply-To: <20120126225231.GA14753@ecki>
On Thu, 26 Jan 2012 23:52:31 +0100, Clemens Buchacher <drizzd@aon.at> wrote:
> How about removing the profile-all target and making it a build option
> instead? To enable it, do the usual:
> (...)
> ifdef PROFILE_BUILD
> all:
> $(MAKE) CFLAGS=... -fprofile-generate ... all-one
> $(MAKE) CFLAGS=... -fprofile-use ... all-one
> else
> all: all-one
> endif
>
> and each previous instance of 'all' replaced with 'all-one'.
Not quite. test: and install: should depend on 'all', otherwise making
them without doing 'make all' first will test/install an unprofiled Git.
So 'all' with profiling should be today's profile-all, which should not
throw away the build and start over. It can create some files to mark
how far it has gotten instead. And profile-generate currently uses
'test' which would recurse, it needs another internal test target.
Not sure if it is worth it. Something like this, perhaps. Except I
have not thought about how this interacts with the coverage targets.
# Final targets
ifdef PROFILE_BUILD
all:: profile-all
test: profile-test
install: profile-install
else
all:: all-one
test: test-one
install: install-one
endif
# Profiling
#
# Note: If profiling (the test phase) failed halfway through but you
# still want to use the partial profile results to build Git, you can
# touch p-gen.stamp
# and then 'make all' again.
profile-all: p-use.stamp
profile-gen p-gen.stamp:
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all-one
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test-one
touch p-gen.stamp
profile-use p-use.stamp: p-gen.stamp
$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all-one
touch p-use.stamp
profile-test: p-use.stamp
$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" test-one
profile-install: p-use.stamp
$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" install-one
.PHONY: all-one test test-one install install-one
.PHONY: profile-all profile-gen profile-test profile-install profile-clean
Also let 'clean' depend on 'profile-clean' which does
$(RM) p-gen.stamp p-use.stamp.
--
Hallvard
^ permalink raw reply
* commit/from command in git-fast-import
From: Mike Hommey @ 2012-01-27 12:48 UTC (permalink / raw)
To: git
Hi,
I'm trying to make sense of the git fast-import manual page. This is
what it reads, as of git 1.7.8.3, from Debian unstable:
Omitting the from command in the first commit of a new branch will
cause fast-import to create that commit with no ancestor. This
tends to be desired only for the initial commit of a project. If
the frontend creates all files from scratch when making a new
branch, a merge command may be used instead of from to start the
commit with an empty tree. Omitting the from command on existing
branches is usually desired, as the current commit on that branch
is automatically assumed to be the first ancestor of the new
commit.
When I do create a commit on a given branch with a stream like:
commit refs/heads/branch
author ...
committer ...
data <<EOF
Commit message
EOF
deleteall
All I get is this warning:
warning: Not updating refs/heads/branch (new tip new_sha1
does not contain old_sha1)
And the branch only has one commit, which is the one I just created.
On the other hand, if I add a "from" instruction in the above stream,
I have the expected branch history.
Arguably, this may be related to my use of deleteall, but nothing in the
deleteall description suggests this would happen.
Is it an expected behaviour and a lack of proper documentation, or is it
a bug in git fast-import ?
Mike
^ permalink raw reply
* Re: commit/from command in git-fast-import
From: David Barr @ 2012-01-27 14:00 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20120127124837.GA24084@glandium.org>
On Fri, Jan 27, 2012 at 11:48 PM, Mike Hommey <mh@glandium.org> wrote:
> When I do create a commit on a given branch with a stream like:
> commit refs/heads/branch
> author ...
> committer ...
> data <<EOF
> Commit message
> EOF
> deleteall
>
> All I get is this warning:
> warning: Not updating refs/heads/branch (new tip new_sha1
> does not contain old_sha1)
>
> And the branch only has one commit, which is the one I just created.
> On the other hand, if I add a "from" instruction in the above stream,
> I have the expected branch history.
This is precisely the expected behavior.
If 'from' is omitted, the resulting commit has no preceding history.
On the other hand, what you want is to specify the parent so that
there is a continuation of history.
I hope this helps.
--
David Barr
^ permalink raw reply
* git-gui Ctrl-U (unstage) broken
From: Victor Engmark @ 2012-01-27 14:03 UTC (permalink / raw)
To: git
Using the git-gui available with the default Ubuntu 10.10 repos, I'm
not able to unstage files with the default keyboard shortcut. To
reproduce:
1. Change a file in the repository
2. Run `git gui`
3. Stage the changed file
4. Select the changed file in the "Staged Changes (Will Commit)" list
5. Click Ctrl-U
Expected outcome: The selected file should be unstaged.
Actual outcome: Nothing at all changes in the GUI.
Verified that other keyboard shortcuts work: Ctrl-T, Ctrl-I, Ctrl--,
Ctrl-+, F5. These (except Ctrl-T, obviously) were tested in* both the
"Unstaged Changes" and "Staged Changes (Will Commit)" listsp
* That is, after focusing a single element in that list.
Version info:
git-gui version 0.12.0.64.g89d6
git version 1.7.1
Tcl/Tk version 8.5.8
Aspell 0.60.6, en_US
Cheers,
V
^ permalink raw reply
* Re: commit/from command in git-fast-import
From: Mike Hommey @ 2012-01-27 14:08 UTC (permalink / raw)
To: David Barr; +Cc: git
In-Reply-To: <CAFfmPPPYc=9BdwuE+ANiHKrFk+_7aXDgnMv3fHxVmF0ttZu8bA@mail.gmail.com>
On Sat, Jan 28, 2012 at 01:00:17AM +1100, David Barr wrote:
> On Fri, Jan 27, 2012 at 11:48 PM, Mike Hommey <mh@glandium.org> wrote:
> > When I do create a commit on a given branch with a stream like:
> > commit refs/heads/branch
> > author ...
> > committer ...
> > data <<EOF
> > Commit message
> > EOF
> > deleteall
> >
> > All I get is this warning:
> > warning: Not updating refs/heads/branch (new tip new_sha1
> > does not contain old_sha1)
> >
> > And the branch only has one commit, which is the one I just created.
> > On the other hand, if I add a "from" instruction in the above stream,
> > I have the expected branch history.
>
> This is precisely the expected behavior.
> If 'from' is omitted, the resulting commit has no preceding history.
> On the other hand, what you want is to specify the parent so that
> there is a continuation of history.
This is however not what the manpage suggests in what I quoted in my
message:
Omitting the from command on existing branches is usually desired, as
the current commit on that branch is automatically assumed to be the
first ancestor of the new commit.
Mike
^ permalink raw reply
* gitweb showing slash r at the end of line
From: Ondra Medek @ 2012-01-27 14:19 UTC (permalink / raw)
To: git
Hi,
we have gitweb running on Linux box. Some files have Windows line ending
(CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
in the end of each line, which is annoying. I have creates a simple patch to
avoid this. It adds just one line. I am not sure if the regexp should
contain 'g' switch in the end. Also, not sure if there shoul be some config
option to switch on/off this?
Cheers
Ondra
---
gitweb/gitweb.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..92175bc 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1500,6 +1500,7 @@ sub esc_html {
if ($opts{'-nbsp'}) {
$str =~ s/ / /g;
}
+ $str =~ s/\r$//;
$str =~ s|([[:cntrl:]])|(($1 ne "\t") ? quot_cec($1) : $1)|eg;
return $str;
}
--
1.7.8.4
--
View this message in context: http://git.661346.n2.nabble.com/gitweb-showing-slash-r-at-the-end-of-line-tp7229895p7229895.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply related
* Re: [PATCH] git-completion: workaround zsh COMPREPLY bug
From: Felipe Contreras @ 2012-01-27 16:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy
In-Reply-To: <7v4nvi2kgq.fsf@alter.siamese.dyndns.org>
On Fri, Jan 27, 2012 at 12:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> zsh adds a backslash (foo\ ) for each item in the COMPREPLY array if IFS
>> doesn't contain spaces. This issue has been reported[1], but there is no
>> solution yet.
>> ...
>> Once zsh is fixed, we should conditionally disable this workaround to
>> have the same benefits as bash users.
>>
>> [1] http://www.zsh.org/mla/workers/2012/msg00053.html
>> [2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=2e25dfb8fd38dbef0a306282ffab1d343ce3ad8d
>
> That 2e25dfb8 only says:
>
> Rocky Bernstein: 29135 (plus tweaks): compgen -W in bash completion
>
> without any explanation, which is not very useful.
Yeah, they development practices leaves a lot to be desired.
> Do you have a bug tracker ID or something for [1] above, with which I can
> amend the patch as Matthieu suggests?
I don't think there's something like that, but here's the original discussion:
http://thread.gmane.org/gmane.comp.shells.zsh.devel/22541
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Jens Lehmann @ 2012-01-27 17:03 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120126225140.GB12855@sigill.intra.peff.net>
Am 26.01.2012 23:51, schrieb Jeff King:
> However, I didn't think about the fact that git-submodule.sh would be
> calling git-config separately, and that is accidentally changed by my
> patch. Even if we changed git-submodule to use "git config
> --no-includes" that would break any third-party scripts that use "git
> config" to read git-like files.
>
> But it would be nice for callers doing "git config foo.bar" to get the
> includes by default. So maybe the right rule is:
>
> 1. In C:
> a. git_config() respects includes automatically.
> b. other callers do not do so automatically (e.g., gitmodules via
> submodule.c).
>
> (i.e., what is implemented by this patch)
>
> 2. Callers of git-config:
> a. respect includes for lookup that checks all of the "normal"
> config spots in sequence: .git/config, ~/.gitconfig, and
> /etc/gitconfig. These are the shell equivalent of calling
> git_config().
>
> b. when we are looking in a specific file (via GIT_CONFIG or "git
> config -f"), do not respect includes (but allow --includes if
> the caller chooses). This specific file may be something like
> .gitmodules. Or perhaps somebody is saying "no, I really just
> want to know what is in _this_ file, not what the config
> ecosystem tells me in general".
>
> And then because of 1a and 2a, most programs should Just Work without
> any changes, but because of 1b and 2b, any special uses will have to
> decide manually whether they would want to allow includes.
>
> Does that make sense?
To me it really does. It lets submodule.c:gitmodules_config and
"git config -f .gitmodules" behave in the same way, which is very
important for consistent behavior between the submodule script and
the submodule functionality that is already handled in c. And I don't
know of a use case for includes in .gitmodules (the main reason for
adding includes seems to be to enable users to have configuration
stored in the repo, which the .gitmodules file already is. And if it
is about having out of repo configuration blended in, .gitmodules
settings are always overridden by those in .git/config, and you can
use includes there).
The only thing I'm not so sure about is the GIT_CONFIG case. I don't
know if using this is rather a "I moved my config there, but please
respect includes there too" or a "I want git config to look at a
completely different file" use case. Probably both. But also the
question of where to look for relative paths seems not so easy to
answer for the GIT_CONFIG case, so it might be best to just disable
includes there too.
^ permalink raw reply
* Re: [RFC/PATCH 0/4] config include directives
From: Jeff King @ 2012-01-27 17:34 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <CACBZZX59sur4_61LkN_sMOvXQ4Jdnt1P8O-UOgm0SooBQpjFdQ@mail.gmail.com>
On Fri, Jan 27, 2012 at 10:51:34AM +0100, Ævar Arnfjörð Bjarmason wrote:
> If you write the function like that it means your patch series just
> works since values encountered later will override earlier ones, but
> have you checked git's code to make sure we don't have anything like:
>
> static int ignore_add_errors_is_set = 0;
> static int add_config(const char *var, const char *value, void *cb)
> {
> if (!ignore_add_errors_is_set &&
> (!strcmp(var, "add.ignoreerrors") ||
> !strcmp(var, "add.ignore-errors"))) {
> ignore_add_errors = git_config_bool(var, value);
> ignore_add_errors_is_set = 1;
> return 0;
> }
> return git_default_config(var, value, cb);
> }
>
> Which would mean that the include config support would be silently
> ignored.
I'm not sure what the issue is. If you write code like this, it will
already ignore the second invocation when it is found later in the same
file, or when it is found in a later file (i.e., in both .git/config and
.gitconfig). So I don't think includes introduce a new problem with
respect to code like this (and no, I didn't check exhaustively, but I
don't recall seeing code like this in git).
A bigger potential problem is multi-key values that form lists. For
example, I cannot use a later "remote.foo.url" line to override an
earlier one; instead, it gets appended to the list of URLs for "foo".
In practice, it's not a problem because the list-like options don't tend
to be found in multiple places. And again, this is not a new problem of
includes, since we already handle multiple files.
Accidentally including the same file twice would cause duplicates for
multi-key values. But I'm going to take Junio's suggestion to avoid
including the same file twice (which also prevents infinite loops due to
cycles).
-Peff
^ permalink raw reply
* [PATCH] Revert "gitweb: Time::HiRes is in core for Perl 5.8"
From: Jakub Narebski @ 2012-01-27 17:45 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Hallvard Breien Furuseth, git
In-Reply-To: <CACBZZX4cjcY5d3mPJAV+rbSTqCEUOrF=_dd3ny_jSM++G-Bg1Q@mail.gmail.com>
On Mon, 23 Jan 2012, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jan 23, 2012 at 05:50, Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> wrote:
> >
> > t9500-gitweb-standalone-no-errors fails: Git 1.7.9.rc2/1.7.8.4, RHEL
> > 6.2, Perl 5.10.1. Reverting 3962f1d756ab41c1d180e35483d1c8dffe51e0d1
> > fixes it. The commit expects Time::HiRes to be present, but RedHat
> > has split it out to a separate RPM perl-Time-HiRes. Better add a
> > comment about that, so it doesn't get re-reverted.
> >
> > Or pacify the test and expect gitweb@RHEL-users to install the RPM:
[...]
> This doesn't actually fix the issue, it only sweeps it under the rug
> by making the tests pass, gitweb will still fail to compile on Red
> Hat once installed.
I think you meant "fail to run" here.
> I think the right solution is to partially revert
> 3962f1d756ab41c1d180e35483d1c8dffe51e0d1, but add a comment in the
> code indicating that it's to deal with RedHat's broken fork of Perl.
I have added comment in commit message, but not in code. I wonder if
it would be enough.
> However even if it's required in an eval it might still fail at
> runtime in the reset_timer() function, which'll need to deal with it
> too.
It shouldn't; everything else related to timer is protected with
'if defined $t0', which is false if Time::HiRes module is not available.
Here is the patch
-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] Revert "gitweb: Time::HiRes is in core for Perl 5.8"
This reverts commit 3962f1d756ab41c1d180e35483d1c8dffe51e0d1.
Though Time::HiRes is a core Perl module, it doesn't necessarily mean
that it is included in 'perl' package, and that it is installed if
Perl is installed.
For example RedHat has split it out to a separate RPM perl-Time-HiRes.
Noticed-by: Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no>
Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jakub Narębski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..c86224a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,10 +17,12 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
-use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
-our $t0 = [ gettimeofday() ];
+our $t0;
+if (eval { require Time::HiRes; 1; }) {
+ $t0 = [Time::HiRes::gettimeofday()];
+}
our $number_of_git_cmds = 0;
BEGIN {
@@ -1142,7 +1144,7 @@ sub dispatch {
}
sub reset_timer {
- our $t0 = [ gettimeofday() ]
+ our $t0 = [Time::HiRes::gettimeofday()]
if defined $t0;
our $number_of_git_cmds = 0;
}
@@ -3974,7 +3976,7 @@ sub git_footer_html {
print "<div id=\"generating_info\">\n";
print 'This page took '.
'<span id="generating_time" class="time_span">'.
- tv_interval($t0, [ gettimeofday() ]).
+ Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).
' seconds </span>'.
' and '.
'<span id="generating_cmd">'.
@@ -6253,7 +6255,7 @@ sub git_blame_common {
print 'END';
if (defined $t0 && gitweb_check_feature('timed')) {
print ' '.
- tv_interval($t0, [ gettimeofday() ]).
+ Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).
' '.$number_of_git_cmds;
}
print "\n";
--
1.7.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