* 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: 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: 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: [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: [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 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: 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 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: [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
* [PATCH] gitweb: add pf= to limit project list to a subdirectory
From: Bernhard R. Link @ 2012-01-26 14:45 UTC (permalink / raw)
To: git
This commit changes the project listings (project_list, project_index
and opml) to limit the output to only projects in a subdirectory if the
new optional parameter ?pf=directory name is used.
It uses the infrastructure already there for 'forks' (which also filters
projects but needs a project called like the filter directory to work).
This feature is disabled if strict_export is used and there is no
projects_list to avoid showing more than intended.
Without strict_export enabled this change should not show any projects
one could not get details from anyway. So if the validate_pathname
checks are not sufficient this would at most make it easier to get a
list of viewable content.
Reusing $project instead of adding a new parameter would have been
nicer from a UI point-of-view (including PATH_INFO support) but
complicate the $project validating code that is currently being
used to ensure nothing is exported that should not be viewable.
Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
As most parameters are not documented in documentation/gitweb.txt,
I did not add documentation for this one either.
gitweb/gitweb.perl | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..00dd79e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -760,6 +760,7 @@ our @cgi_param_mapping = (
search_use_regexp => "sr",
ctag => "by_tag",
diff_style => "ds",
+ project_filter => "pf",
# this must be last entry (for manipulation from JavaScript)
javascript => "js"
);
@@ -976,7 +977,7 @@ sub evaluate_path_info {
our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,
$hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,
- $searchtext, $search_regexp);
+ $searchtext, $search_regexp, $project_filter);
sub evaluate_and_validate_params {
our $action = $input_params{'action'};
if (defined $action) {
@@ -994,6 +995,16 @@ sub evaluate_and_validate_params {
}
}
+ our $project_filter = $input_params{'project_filter'};
+ if (defined $project_filter) {
+ if ($strict_export and -d $projects_list) {
+ die_error(404, "project_filter disabled");
+ }
+ if (!validate_pathname($project_filter)) {
+ die_error(404, "Invalid project_filter parameter");
+ }
+ }
+
our $file_name = $input_params{'file_name'};
if (defined $file_name) {
if (!validate_pathname($file_name)) {
@@ -3962,6 +3973,13 @@ sub git_footer_html {
-class => $feed_class}, $format)."\n";
}
+ } elsif (defined $project_filter) {
+ print $cgi->a({-href => href(project=>undef, action=>"opml",
+ project_filter => $project_filter),
+ -class => $feed_class}, "OPML") . " ";
+ print $cgi->a({-href => href(project=>undef, action=>"project_index",
+ project_filter => $project_filter),
+ -class => $feed_class}, "TXT") . "\n";
} else {
print $cgi->a({-href => href(project=>undef, action=>"opml"),
-class => $feed_class}, "OPML") . " ";
@@ -5979,7 +5997,7 @@ sub git_project_list {
die_error(400, "Unknown order parameter");
}
- my @list = git_get_projects_list();
+ my @list = git_get_projects_list($project_filter);
if (!@list) {
die_error(404, "No projects found");
}
@@ -6018,7 +6036,7 @@ sub git_forks {
}
sub git_project_index {
- my @projects = git_get_projects_list();
+ my @projects = git_get_projects_list($project_filter);
if (!@projects) {
die_error(404, "No projects found");
}
@@ -7855,7 +7873,7 @@ sub git_atom {
}
sub git_opml {
- my @list = git_get_projects_list();
+ my @list = git_get_projects_list($project_filter);
if (!@list) {
die_error(404, "No projects found");
}
--
1.7.8.3
^ permalink raw reply related
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-01-27 6:35 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, Conrad Irwin, git, Nguyen Thai Ngoc Duy,
Dov Grobgeld
In-Reply-To: <4F21831C.7060609@alum.mit.edu>
On Thu, Jan 26, 2012 at 05:45:16PM +0100, Michael Haggerty wrote:
> I think decisions such as whether to include an imported module in "git
> diff" output is a personal preference and should not be decided at the
> level of the git project.
You're right. I thought of it as an annotation that the project could
mark via .gitattributes, or the user could mark via .git/info/attributes.
But that is not following the right split of responsibility for
attributes and config. The attributes should annotate "this isn't really
part of the regular git code base" or "this is really part of the
nedmalloc codebase". And then the _config_ should say "when I am
grepping, I am not interested in nedmalloc". I.e.:
# mark a set of paths with an attribute
echo "compat/nedmalloc external" >>.gitattributes
# and then ignore that attribute for this grep
git grep --exclude-attr=external
# or for all greps
git config --add grep.exclude external
and git doesn't even have to care about what the attribute is called.
It's between the project and the user how they want to annotate their
files, and how they want to feed them to grep.
Or any other program, for that matter. I wonder if this could also be a
more powerful way of grouping files to be included or excluded from diff
pathspecs. Something like (and I'm just talking off the top of my head,
so there may be some syntactic conflicts here):
# annotate some files
cat >>.gitattributes <<-\EOF
t/t????-*.sh test-script
t/lib-*.sh test-script
t/test-lib.sh test-script
EOF
# and then consider the tagged files to be a group, and look only at
# that group
git log :attr:test-script
# ditto, but imagine we had the negative pathspecs Duy has proposed
git log :~attr:test-script
That seems kind of cool to me. But maybe it is getting into crazy
over-engineering. I like the idea that we don't need a new option to
grep or diff; rather it is simply a new syntax for mentioning paths.
> The in-tree .gitattributes files should, by and large, just *describe*
> the files and leave it to users to associate policies with the tags
> (or at least make it possible for users to override the policies) via
> .git/info/attributes. For example, the repository could set an
> "external=nedmalloc" attribute on all files under compat/nedmalloc,
> and users could themselves configure a macro "[attr]external -diff
> -grep" (or maybe something like "[attr]external=nedmalloc -diff
> -grep") if that is their preference.
So obviously I took what you were saying here and ran with it above. But
I do disagree with one thing here: the attributes should be giving some
tag to the paths, but the actual decision about whether to grep should
be part of the _config_. That's the usual split we have for all of the
other attributes, and I think it makes sense and has worked well.
> Is it really common to want to use the same argument on multiple macros
> without also wanting to set other things specifically? If not, then
> there is not much reason to complicate macros with argument support.
I dunno. I admit my attribute usage tends to just match by extension,
and I generally only have one or two such lines.
> For example, I do something like
>
> [attr]type-python type=python text diff=python check-ws
> *.py type-python
>
> [attr]type-makefile type=makefile text diff check-ws -check-tab
> Makefile.* type-makefile
>
> for the main file types in my repository, and it is not very cumbersome.
I think it's not a big deal if you are making your own macros. I was
more concerned that people would want to use the "binary" macro to get
the "-grep" automagic, but could not do so because they don't want
"-diff", but rather "diff=foo".
Anyway, after reading your response and thinking on it more, I think
"-grep" is totally the wrong way to go. If the files are marked binary,
then grep should be respecting "-diff" or the "diff.*.binary" config. If
we want to do more advanced exclusion, then the right place for that is
the config file (or the weird :attr pathspec thing I mentioned above).
> "type-python" and "type=python" seem redundant but they are not.
> "type-python" is needed so that it can be used as a macro.
> "type=python" makes it easier to inquire about the type of a file using
> something like "git check-attr type -- PATH" rather than having to
> inquire about each possible type-* attribute. It might be nice to
> support a slightly extended macro definition syntax like
>
> [attr]type=python text diff=python check-ws
> *.py type=python
>
> [attr]type=makefile text diff check-ws -check-tab
> Makefile.* type=makefile
>
> (i.e., macros that are only triggered for particular values of an
> attribute).
I don't think there's any semantic reason why that is not workable. It's
simply not syntactically allowed at this point.
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Jeff King @ 2012-01-27 5:59 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8DPcgJtw_xxqZ2pOtpV-w=PAVQ7uHs+CJ+ynECYdL50og@mail.gmail.com>
On Fri, Jan 27, 2012 at 11:01:00AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Thu, Jan 26, 2012 at 2:42 PM, Jeff King <peff@peff.net> wrote:
> > +Security Considerations
> > +~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Because git configuration may cause git to execute arbitrary shell
> > +commands, it is important to verify any configuration you receive over
> > +the network. In particular, it is not a good idea to point `include.ref`
> > +directly at a remote tracking branch like `origin/master:shared-config`.
> > +After a fetch, you have no way of inspecting the shared-config you have
> > +just received without running git (and thus respecting the downloaded
> > +config). Instead, you can create a local tag representing the last
> > +verified version of the config, and only update the tag after inspecting
> > +any new content.
>
> It may be a good idea to tell users the ref include.ref points to has
> been updated at the end of git-fetch. Showing a diff is even better.
I really didn't want to have to let other parts of git know or care
about this mechanism. At least not for now. In the long run, I have no
problem with some porcelain growing up around the feature to make it
simpler to use. But I'd really rather focus on the bare-bones
functionality for now, see how people use it, and then find ways to
address deficiencies in their workflows once we have data.
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Jeff King @ 2012-01-27 5:57 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Johannes Sixt, git
In-Reply-To: <CACsJy8ASXrmBvxj4DxjGiqYhPkr1Yp02CAyMqKrfyfrzaAw-2g@mail.gmail.com>
On Fri, Jan 27, 2012 at 10:47:29AM +0700, Nguyen Thai Ngoc Duy wrote:
> > What happens if a ref cannot be resolved, for example due to repository
> > corruption? Does git just emit an error and then carries on, or does it
> > always die? Can I run at least git-fsck in such a case?
>
> Moreover, if I specify sha-1 in the config (it's discouraged but not
> forbidden from the code), can git-prune remove the blob?
Yes. I don't think we want to get into connectivity guarantees for
config (because they can be quite complex, and involve files totally
outside the repo). I think it's OK for the user to be responsible for
either using a ref, or making sure that a bare sha1 they point to is
reachable from a ref.
-Peff
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Jeff King @ 2012-01-27 5:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk44d1zww.fsf@alter.siamese.dyndns.org>
On Thu, Jan 26, 2012 at 09:23:59PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > 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?
>
> In short, the "git config" interface defaults to "--no-includes" when
> reading from an explicit file with "-f" and "--includes" otherwise, which
> sounds like a 100% sensible default to me.
Yes, exactly. Thank you for explaining it in about 1/10 the words I
needed to use. :)
I'll put that behavior in the re-roll.
> > How about:
> >
> > The included file is processed immediately, before any other
> > directives from the surrounding file.
> >
> > What I wanted to make clear there is the ordering, which sometimes
> > matters.
>
> Hmm, I think the original is probably easier to read.
OK, then I'll leave it unless somebody else wants to come up with
something brilliant.
-Peff
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Jeff King @ 2012-01-27 5:54 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <4F223115.3050004@alum.mit.edu>
On Fri, Jan 27, 2012 at 06:07:33AM +0100, Michael Haggerty wrote:
> > +struct git_config_include_data {
> > + config_fn_t fn;
> > + void *data;
> > +};
> > +int git_config_include(const char *name, const char *value, void *vdata);
> > +
> > extern const char *config_exclusive_filename;
> >
> > #define MAX_GITNAME (1000)
>
> How about a short comment or two?
I had originally planned to document this somewhat non-intuitive
interface in the config API documentation. But then I noticed we didn't
have such a document, and promptly forgot about documenting.
I'd rather have an API document, but I admit that the thought of
describing the config interface frightens me. It has some nasty corners.
But maybe starting one with the non-scary bits would be better, and then
I could add this to it.
> > +int git_config_include(const char *name, const char *value, void *vdata)
> > +{
> > + const struct git_config_include_data *data = vdata;
> > + const char *type;
> > + int ret;
> > +
> > + /*
> > + * Pass along all values, including "include" directives; this makes it
> > + * possible to query information on the includes themselves.
> > + */
> > + ret = data->fn(name, value, data->data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (prefixcmp(name, "include."))
> > + return ret;
> > + type = strrchr(name, '.') + 1;
> > +
> > + if (!strcmp(type, "path"))
> > + ret = handle_path_include(value, vdata);
> > +
> > + return ret;
> > +}
> > +
>
> Doesn't this code accept all keys of the form "include\.(.*\.)?path"
> (e.g., "include.foo.path")? If that is your intention, then the
> documentation should be fixed. If not, then a single strcmp(name,
> "include.path") would seem sufficient.
It does. I was considering (but haven't yet written) a patch that would
allow for conditional inclusion, like:
[include "foo"]
path = /some/file
where "foo" would be the condition. Specifically, I wanted to enable
includes when certain features were available in the parsing version of
git. For example, the pager.* variables were originally bools, but later
learned to take arbitrary strings. So my config with arbitrary strings
works on modern git, but causes earlier versions of git to barf. I'd
like to be able to do something like:
[include "per-command-pager-strings"]
path = /path/to/my/pager.config
where "per-command-pager-strings" would be a flag known internally to
git versions that support that feature.
I didn't end up implementing it right away, because of course those same
early versions of git also don't know about "include" at all. So using
any include effectively works as a conditional for that particular
feature. But as new incompatible config semantics are added
post-include, they could take advantage of a similar scheme.
So I wanted to leave the code open to adding such a patch later, if and
when it becomes useful. That being said, the code above is wrong.
For my scheme to work, versions of git that handle includes but don't
have the conditional-include patch (if it ever comes) would want to
explicitly disallow includes with subsections.
I'll fix it in the re-roll.
> > + struct git_config_include_data inc;
> > +
> > + inc.fn = fn;
> > + inc.data = data;
> > + fn = git_config_include;
> > + data = &inc;
> >
> > /* Setting $GIT_CONFIG makes git read _only_ the given config file. */
> > if (config_exclusive_filename)
>
> The comment just after your addition should be adjusted, since now "the
> given config file and any files that it includes" are read.
Will do.
-Peff
^ permalink raw reply
* Re: Test t9500 fails if Time::HiRes is missing
From: Junio C Hamano @ 2012-01-27 5:48 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Hallvard Breien Furuseth, git, Jakub Narebski
In-Reply-To: <CACBZZX4cjcY5d3mPJAV+rbSTqCEUOrF=_dd3ny_jSM++G-Bg1Q@mail.gmail.com>
Æ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.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Hopefully, this may resolve itself over time.
INSTALL | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/INSTALL b/INSTALL
index 8120641..6fa83fe 100644
--- a/INSTALL
+++ b/INSTALL
@@ -83,7 +83,11 @@ Issues of note:
- "Perl" version 5.8 or later is needed to use some of the
features (e.g. preparing a partial commit using "git add -i/-p",
interacting with svn repositories with "git svn"). If you can
- live without these, use NO_PERL.
+ live without these, use NO_PERL. Note that recent releases of
+ Redhat/Fedora are reported to ship Perl binary package with some
+ core modules stripped away (see http://lwn.net/Articles/477234/),
+ so you might need to install additional packages other than Perl
+ itself, e.g. Time::HiRes.
- "openssl" library is used by git-imap-send to use IMAP over SSL.
If you don't need it, use NO_OPENSSL.
^ permalink raw reply related
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Jeff King @ 2012-01-27 5:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3a51zlb.fsf@alter.siamese.dyndns.org>
On Thu, Jan 26, 2012 at 09:30:56PM -0800, Junio C Hamano wrote:
> While I do not think origin/meta:config is a sensible default, I actually
> do think that:
>
> [include]
> ref = meta:gitconfig
> [branch "meta"]
> remote = origin
> merge = refs/heads/meta
>
> makes some sense. The earlier example with the in-tree dev_tools/config in
> the same line of history as the usual source material to keep track of
> private changes ("this single user hating it") was not realistic as it
> forbids the user from sharing the rest of the source once she decides to
> fork the config preference.
I don't think having it in-tree makes a difference. I can fork the
regular tree into my config branch, and it contains only my config
changes. If I want to share config changes with people, then I do so by
sharing that branch. But it need not have any impact on the "real"
branch I create from the regular tree. The fact that the rest of the
source files are in the config branch are irrelevant.
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.
-Peff
^ permalink raw reply
* git rebase likes to fail miserably on Mac OS X Lion
From: Joshua Jensen @ 2012-01-27 5:25 UTC (permalink / raw)
To: git@vger.kernel.org
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:
fatal: Unable to create
'/Users/joshua/src/project/.git/index.lock': File exists.
or:
error: Your local changes to the following files would be
overwritten by merge:
^^^ There are no local changes when the rebase begins. This is caused
by the rebase.
I have tried both Git v1.7.5.4 and 1.7.8.4.
I use msysGit for the majority of my Git usage, and I do not run into
this problem there.
Is there anything to be done? Right now, the only workaround I can
think of is to cherry pick changes one at a time as a fake rebase. Ick.
Thanks.
Josh
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Junio C Hamano @ 2012-01-27 5:30 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120127004902.GA15257@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> No, I meant it explicitly to be about this single user hating it. Note
> how the resulting commits are never pushed. It is purely a local
> override, but with the added bonus that history is tracked so you can
> merge in further changes from upstream.
> ...
> It also does allow "[include]ref = origin/meta:gitconfig" if you want to
> live dangerously. I consider that a feature, because it lets the user
> make the security tradeoff they deem appropriate.
While I do not think origin/meta:config is a sensible default, I actually
do think that:
[include]
ref = meta:gitconfig
[branch "meta"]
remote = origin
merge = refs/heads/meta
makes some sense. The earlier example with the in-tree dev_tools/config in
the same line of history as the usual source material to keep track of
private changes ("this single user hating it") was not realistic as it
forbids the user from sharing the rest of the source once she decides to
fork the config preference.
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Junio C Hamano @ 2012-01-27 5:23 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120126225140.GB12855@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 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?
In short, the "git config" interface defaults to "--no-includes" when
reading from an explicit file with "-f" and "--includes" otherwise, which
sounds like a 100% sensible default to me.
> I had a similar thought while writing it, but hoped the sentence after
> (that you snipped) would make it clear.
I think the whole paragraph makes it reasonably clear; I was merely being
pedantic to see if you or others can come up with a clear and simple way
to rephrase it that can also satisfy such pedantry.
> How about:
>
> The included file is processed immediately, before any other
> directives from the surrounding file.
>
> What I wanted to make clear there is the ordering, which sometimes
> matters.
Hmm, I think the original is probably easier to read.
> The one use I think is to bundle a bunch of related config options, and
> then turn them on selectively.
> ...
> but with this patch, you can do:
>
> cat >~/.gitconfig.foo <<-\EOF
> [foo]
> one = 1
> two = 2
> EOF
> git -c include.path=$HOME/.gitconfig.foo blah
That is quite a sensible use case actually.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/4] config: add include directive
From: Michael Haggerty @ 2012-01-27 5:07 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120126073752.GA30474@sigill.intra.peff.net>
On 01/26/2012 08:37 AM, Jeff King wrote:
> [...]
> This patch introduces an include directive for config files.
> It looks like:
>
> [include]
> path = /path/to/file
I like it.
> diff --git a/cache.h b/cache.h
> index 10afd71..21bbb0a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1138,6 +1138,12 @@ extern const char *get_commit_output_encoding(void);
>
> extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
>
> +struct git_config_include_data {
> + config_fn_t fn;
> + void *data;
> +};
> +int git_config_include(const char *name, const char *value, void *vdata);
> +
> extern const char *config_exclusive_filename;
>
> #define MAX_GITNAME (1000)
How about a short comment or two?
> diff --git a/config.c b/config.c
> index 40f9c6d..a6966c1 100644
> --- a/config.c
> +++ b/config.c
> [...]
> +int git_config_include(const char *name, const char *value, void *vdata)
> +{
> + const struct git_config_include_data *data = vdata;
> + const char *type;
> + int ret;
> +
> + /*
> + * Pass along all values, including "include" directives; this makes it
> + * possible to query information on the includes themselves.
> + */
> + ret = data->fn(name, value, data->data);
> + if (ret < 0)
> + return ret;
> +
> + if (prefixcmp(name, "include."))
> + return ret;
> + type = strrchr(name, '.') + 1;
> +
> + if (!strcmp(type, "path"))
> + ret = handle_path_include(value, vdata);
> +
> + return ret;
> +}
> +
Doesn't this code accept all keys of the form "include\.(.*\.)?path"
(e.g., "include.foo.path")? If that is your intention, then the
documentation should be fixed. If not, then a single strcmp(name,
"include.path") would seem sufficient.
> int git_config_early(config_fn_t fn, void *data, const char *repo_config)
> {
> int ret = 0, found = 0;
> const char *home = NULL;
> + struct git_config_include_data inc;
> +
> + inc.fn = fn;
> + inc.data = data;
> + fn = git_config_include;
> + data = &inc;
>
> /* Setting $GIT_CONFIG makes git read _only_ the given config file. */
> if (config_exclusive_filename)
The comment just after your addition should be adjusted, since now "the
given config file and any files that it includes" are read.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: How to migrate a complex directory structure from SVN to GIT?
From: David Barr @ 2012-01-27 4:46 UTC (permalink / raw)
To: Jehan Bing; +Cc: git, Jonathan Nieder, Dmitry Ivankov, Ramkumar Ramachandra
In-Reply-To: <4F1713D4.9000602@orb.com>
> If however you have a more complex layout, you can use "git svn init", then
> edit .git/config to suit your needs, then run "git svn fetch".
> And by "suit your needs", I mean you can add multiple "fetch=..." lines.
> In my case, I ended up having one "fetch=..." for each trunk, branch and
> tag.
> It was not efficient, it took 2 weeks to convert <30k revisions, ~200
> branches/project, ~80 projects, but it works well enough for me.
>
> Jehan
This is why we need to breath life into the git-remote-svn effort.
We should be able to handle imports of that size in minutes.
(Cost of dumping from SVN aside.)
--
David Barr
^ permalink raw reply
* Re: Git svn migration does not work because fatal git checkout updating paths is incompatible with switching branches
From: David Barr @ 2012-01-27 4:34 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Christine Bauers, git
In-Reply-To: <1327518563.31804.82.camel@centaur.lab.cmartin.tk>
Hi,
A lot of work has occurred on the version of svn-fe within the git tree.
Jonathan Nieder is the nominal maintainer of that effort.
He has just requested that the most stable set of changes be merged.
--
David Barr
From Jonathan's pull request:
Junio, please pull
git://repo.or.cz/git/jrn.git svn-fe
to get the following changes since commit
ec014eac0e9e6f30cbbca616090fa2ecf74797e7:
Git 1.7.5 (2011-04-23 23:36:32 -0700)
up to c5bcbcdcfa1e2a1977497cb3a342c0365c8d78d6:
vcs-svn: reset first_commit_done in fast_export_init (2011-06-23
10:04:36 -0500)
On Thu, Jan 26, 2012 at 6:09 AM, Carlos Martín Nieto <cmn@elego.de> wrote:
> On Wed, 2012-01-25 at 19:04 +0100, Christine Bauers wrote:
>> Hi there,
>>
>> I´m trying to migrate a repository from svn to git which branches and
>> tags with the following migration script:
>>
>> git svn clone --no-metadata --stdlayout --A ../users.txt
>> svn://host/svn/project/subproject subproject
>>
>> cd subproject
>> git config svn.authorsfile ../../users.txt
>> git svn fetch
>>
>> git checkout -b branch1 remotes/branch1
>> git checkout -b branch2 remotes/branch2
>> git checkout -b branch3 remotes/branch3
>>
>> git checkout -b src_v1 remotes/tags/src
>> git checkout master
>> git tag src src_v1
>> git branch -D src_v1
>>
>> git checkout -b WebContent_v1 remotes/tags/WebContent
>> git checkout master
>> git tag WebContent WebContent_v1
>> git branch -D WebContent_v1
>>
>> and get the follwoing errors:
>>
>> W: Ignoring error from SVN, path probably does not exist: (160013):
>> Filesystem has no item: Datei nicht gefunden: Revision 8966, Pfad
>> »subproject«
>> W: Do not be alarmed at the above message git-svn is just searching
>> aggressively for old history.
>> This may take a while on large repositories
>> fatal: git checkout: updating paths is incompatible with switching branches.
>> Did you intend to checkout 'remotes/branch1' which can not be resolved
>> as commit?
>> fatal: git checkout: updating paths is incompatible with switching branches.
>> Did you intend to checkout 'remotes/branch2 which can not be resolved as
>> commit?
>> fatal: git checkout: updating paths is incompatible with switching branches.
>> Did you intend to checkout 'remotes/branch3' which can not be resolved
>> as commit?
>> fatal: git checkout: updating paths is incompatible with switching branches.
>> Did you intend to checkout 'remotes/tags/src' which can not be resolved
>> as commit?
>> error: pathspec 'master' did not match any file(s) known to git.
>> fatal: Failed to resolve 'src_v1' as a valid ref.
>> error: branch 'src_v1' not found.
>> fatal: git checkout: updating paths is incompatible with switching branches.
>> Did you intend to checkout 'remotes/tags/WebContent' which can not be
>> resolved as commit?
>> error: pathspec 'master' did not match any file(s) known to git.
>> fatal: Failed to resolve 'WebContent_v1' as a valid ref.
>> error: branch 'WebContent_v1' not found.
>>
>> How do I solve this problem?
>
> First try to figure out where the problem happens. It could be that
> git-svn isn't recognising the branches properly, or that the layout
> isn't what it expects or any number of things.
>
> What layout does the repo have? Does it correspond to what git-svn is
> expecting? All those error messages come from the fact that you're
> telling git some starting points that it can't find. Make sure those
> exist and they have the name you're giving. What does `git branch -a`
> say? You're presumably not giving us the real names, so we can't tell if
> there are problems there.
>
> If you're looking to migrate completely, something like
> svn-dump-fast-export ( https://github.com/barrbrain/svn-dump-fast-export
> ) might get you there better.
>
> cmn
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Nguyen Thai Ngoc Duy @ 2012-01-27 4:01 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120126074208.GD30474@sigill.intra.peff.net>
On Thu, Jan 26, 2012 at 2:42 PM, Jeff King <peff@peff.net> wrote:
> +Security Considerations
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Because git configuration may cause git to execute arbitrary shell
> +commands, it is important to verify any configuration you receive over
> +the network. In particular, it is not a good idea to point `include.ref`
> +directly at a remote tracking branch like `origin/master:shared-config`.
> +After a fetch, you have no way of inspecting the shared-config you have
> +just received without running git (and thus respecting the downloaded
> +config). Instead, you can create a local tag representing the last
> +verified version of the config, and only update the tag after inspecting
> +any new content.
It may be a good idea to tell users the ref include.ref points to has
been updated at the end of git-fetch. Showing a diff is even better.
--
Duy
^ permalink raw reply
* Re: [PATCH 4/4] config: allow including config from repository blobs
From: Nguyen Thai Ngoc Duy @ 2012-01-27 3:47 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git
In-Reply-To: <4F211C0C.7060400@viscovery.net>
On Thu, Jan 26, 2012 at 4:25 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 1/26/2012 8:42, schrieb Jeff King:
>> +static int handle_ref_include(const char *ref, void *data)
>> +{
>> + unsigned char sha1[20];
>> + char *buf;
>> + unsigned long size;
>> + enum object_type type;
>> + int ret;
>> +
>> + if (get_sha1(ref, sha1))
>> + return 0;
>> + buf = read_sha1_file(sha1, &type, &size);
>> + if (!buf)
>> + return error("unable to read include ref '%s'", ref);
>> + if (type != OBJ_BLOB)
>> + return error("include ref '%s' is not a blob", ref);
>> +
>> + ret = git_config_from_buffer(git_config_include, data, ref, buf, size);
>> + free(buf);
>> + return ret;
>> +}
>
> What happens if a ref cannot be resolved, for example due to repository
> corruption? Does git just emit an error and then carries on, or does it
> always die? Can I run at least git-fsck in such a case?
Moreover, if I specify sha-1 in the config (it's discouraged but not
forbidden from the code), can git-prune remove the blob?
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox