* Different svn-id URLs in history
From: Knut Eldhuset @ 2008-10-07 10:58 UTC (permalink / raw)
To: git
Hi,
After cloning my svn repository, I notice that the svn-id URL is
different when going back in history:
git-svn-id: https://server/trunk@300
vs
git-svn-id: https://server/trunk/some_folder/project/src@250
If i checkout the first version, I get a working copy that has the
folder "some_folder" in it. If I checkout the latter version, I get a
working copy with the contents of the folder "src". Why is this? I
thought I always was supposed to get a wc with the whole tree.
Regards,
Knut Eldhuset
^ permalink raw reply
* Re: [PATCH] gitweb: refactor input parameters parse/validation
From: Jakub Narebski @ 2008-10-07 10:57 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Shawn O. Pearce, Matthias Lederhofer
In-Reply-To: <1223054356-17643-1-git-send-email-giuseppe.bilotta@gmail.com>
On Fri, 3 Oct 2008, Giuseppe Bilotta wrote:
> Since input parameters can be obtained both from CGI parameters and
> PATH_INFO, we would like most of the code to be agnostic about the way
> parameters were retrieved. We thus collect all the parameters into the
> new %input_params hash, delaying validation after the collection is
> completed.
>
> Although the kludge removal is minimal at the moment, it makes life much
> easier for future expansions such as more extensive PATH_INFO use or
> other form of input (e.g. command-line support).
Looks good, beside double validation of parameters taken from path_info
(once in evaluate_path_info(), and once when setting 'action' variables).
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>
> As recommended by Jakub, I reworked the input parameter collection refactoring
> into a separate patch preluding to rather than intermixed with the PATH_INFO
> enhancement work.
Thanks.
> gitweb/gitweb.perl | 304 +++++++++++++++++++++++++++++-----------------------
> 1 files changed, 171 insertions(+), 133 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 83f810a..329f789 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -29,7 +29,9 @@ our $my_uri = $cgi->url(-absolute => 1);
>
> # if we're called with PATH_INFO, we have to strip that
> # from the URL to find our real URL
> -if (my $path_info = $ENV{"PATH_INFO"}) {
> +# we make $path_info global because it's also used later on
> +my $path_info = $ENV{"PATH_INFO"};
> +if ($path_info) {
> $my_url =~ s,\Q$path_info\E$,,;
> $my_uri =~ s,\Q$path_info\E$,,;
> }
I think we could have left it as it was, i.e. have $path_info be
a local variable. Unless there is something I don't see.
[...]
> +# input parameters are stored with the long parameter name as key, so we need
> +# the name -> CGI key mapping here. This will also be used in the href
> +# subroutine to convert parameters to their CGI equivalent.
Actually href() subroutine would be called many times, while parsing of
input params happens only once; that is why it is longname => shortname
hash, and not the reverse (shortname => longname). Also it does not
result in loss of performance as we parse all input params anyway.
This explanation might be obvious, or could be put in comment
(increasing comment to code count ;-))
> +my @cgi_param_mapping = (
> +my %cgi_param_mapping = @cgi_param_mapping;
> +my %actions = (
Perhaps %allowed_options should also be there, to have everything about
input params in single place... or perhaps not...
> +# fill %input_params with the CGI parameters. All values except for 'opt'
> +# should be single values, but opt can be an array. We should probably
> +# build an array of parameters that can be multi-valued, but since for the time
> +# being it's only this one, we just single it out
> +while (my ($name, $symbol) = each %cgi_param_mapping) {
> + if ($symbol eq 'opt') {
> + $input_params{$name} = [$cgi->param($symbol)];
I would personally use "[ $cgi->param($symbol) ]" instead for better
(IMVHO) visibility, but this is just a matter of taste.
> + } else {
> + $input_params{$name} = $cgi->param($symbol);
> + }
> +}
Nice. I guess that you have checked that you caught all
$cgi->param(...) calls, and there aren't any beside those above...
Now we don't have mdiff, following code movement; tool which was at
beginnings of content movement detection in early days of git-blame.
Therefore I have moved the original code, to easier check the changes
(separating them from code movement).
> -# now read PATH_INFO and use it as alternative to parameters
> -sub evaluate_path_info {
> - return if defined $project;
> - my $path_info = $ENV{"PATH_INFO"};
> - return if !$path_info;
> - $path_info =~ s,^/+,,;
> - return if !$path_info;
> - # find which part of PATH_INFO is project
> - $project = $path_info;
> - $project =~ s,/+$,,;
> - while ($project && !check_head_link("$projectroot/$project")) {
> - $project =~ s,/*[^/]*$,,;
> - }
> - # validate project
> - $project = validate_pathname($project);
> - if (!$project ||
> - ($export_ok && !-e "$projectroot/$project/$export_ok") ||
> - ($strict_export && !project_in_list($project))) {
> - undef $project;
> - return;
> - }
> - # do not change any parameters if an action is given using the query string
> - return if $action;
> - $path_info =~ s,^\Q$project\E/*,,;
> - my ($refname, $pathname) = split(/:/, $path_info, 2);
> - if (defined $pathname) {
> - # we got "project.git/branch:filename" or "project.git/branch:dir/"
> - # we could use git_get_type(branch:pathname), but it needs $git_dir
> - $pathname =~ s,^/+,,;
> - if (!$pathname || substr($pathname, -1) eq "/") {
> - $action ||= "tree";
> - $pathname =~ s,/$,,;
> - } else {
> - $action ||= "blob_plain";
> - }
> - $hash_base ||= validate_refname($refname);
> - $file_name ||= validate_pathname($pathname);
> - } elsif (defined $refname) {
> - # we got "project.git/branch"
> - $action ||= "shortlog";
> - $hash ||= validate_refname($refname);
> - }
> -}
> -evaluate_path_info();
> +# now read PATH_INFO and update the parameter list for missing parameters
> +sub evaluate_path_info {
> + return if defined $input_params{'project'};
I was 'my $path_info = $ENV{"PATH_INFO"};' there, when $path_info
wasn't global variable. Any advantages to having it global?
(This is just a minor thing, not worth resending patch over, I think).
> + return if !$path_info;
> + $path_info =~ s,^/+,,;
> + return if !$path_info;
> +
Nice adding lines to separate parts of code.
> + # find which part of PATH_INFO is project
> + my $project = $path_info;
Hmmm... now $project is local (lexically) here.
> + $project =~ s,/+$,,;
> + while ($project && !check_head_link("$projectroot/$project")) {
> + $project =~ s,/*[^/]*$,,;
> + }
> + # validate project
> + $project = validate_project($project);
I'm not sure if it is worth worrying over, but I think you repeat
check_head_link() check here.
[After examining code further]. But I think you do double validation;
once you do it here, and once you do it copying to global variables
such as $action or $project, and double checking check_head_link()
won't be easy to avoid; fortunately it is cheap filesystem-level check
(might be slow only when stat is extremely slow, and is not cached).
> + return unless $project;
> + $input_params{'project'} = $project;
> +
> + # do not change any parameters if an action is given using the query string
> + return if $input_params{'action'};
> + $path_info =~ s,^\Q$project\E/*,,;
Hmmm... I wonder if it is good idea. It was done in commit 645927c
(gitweb: fix warnings in PATH_INFO code and add export_ok/strict_export)
by Matthias Lederhofer, but I don't see why we do not remove project
from path_info if action is set. But this is belongs probably to
independent code cleanup (if it is needed), so don't worry about that.
Perhaps it would be good to add empty line here before beginning of
'hash' and 'hash_base:file_name' handling.
> + my ($refname, $pathname) = split(/:/, $path_info, 2);
> + if (defined $pathname) {
> + # we got "project.git/branch:filename" or "project.git/branch:dir/"
> + # we could use git_get_type(branch:pathname), but it needs $git_dir
Additionally git_get_type(<extended sha1>) is additional call to git
(additional fork) currently; that might change with gitweb caching code,
which uses permanent connection to 'git cat-file --batch/--batch-check'
for that.
> + $pathname =~ s,^/+,,;
> + if (!$pathname || substr($pathname, -1) eq "/") {
> + $input_params{'action'} = "tree";
> + $pathname =~ s,/$,,;
> + } else {
> + $input_params{'action'} = "blob_plain";
> + }
> + $input_params{'hash_base'} ||= validate_refname($refname);
> + $input_params{'file_name'} ||= validate_pathname($pathname);
> + } elsif (defined $refname) {
> + # we got "project.git/branch"
> + $input_params{'action'} = "shortlog";
> + $input_params{'hash'} ||= validate_refname($refname);
> + }
> +}
Nice defensive programming with '||=' here for setting %input_params.
[After examining code further]. But I think you do double validation;
see comment below.
> +evaluate_path_info();
> +
> +our $action = $input_params{'action'};
> if (defined $action) {
> - if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> + if (!validate_action($action)) {
> die_error(400, "Invalid action parameter");
> }
> }
Hmm... don't you do double validation now? Once in evaluate_path_info,
and once copying to global variables like $action?
Very nice moving param validation for 'a'/'action' parameter to
validate_action() subroutine.
> # parameters which are pathnames
> -our $project = $cgi->param('p');
> +our $project = $input_params{'project'};
> if (defined $project) {
> - if (!validate_pathname($project) ||
> - !(-d "$projectroot/$project") ||
> - !check_head_link("$projectroot/$project") ||
> - ($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
> - ($strict_export && !project_in_list($project))) {
> + if (!validate_project($project)) {
> undef $project;
> die_error(404, "No such project");
> }
> }
Nice using validate_project() here.
> -our @extra_options = $cgi->param('opt');
> -if (defined @extra_options) {
> - foreach my $opt (@extra_options) {
> - if (not exists $allowed_options{$opt}) {
> - die_error(400, "Invalid option parameter");
> - }
> - if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
> - die_error(400, "Invalid option parameter for this action");
> - }
> +our @extra_options = @{$input_params{'extra_options'}};
> +# @extra_options is always defined, since it can only be (currently) set from
> +# CGI, and $cgi->param() returns the empty array in array context
...if param was not set.
Thanks for catching that.
> +foreach my $opt (@extra_options) {
> + if (not exists $allowed_options{$opt}) {
> + die_error(400, "Invalid option parameter");
> + }
> + if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
> + die_error(400, "Invalid option parameter for this action");
> }
> }
> @@ -470,23 +587,23 @@ if (defined $hash_parent_base) {
> }
>
> # other parameters
> -our $page = $cgi->param('pg');
> +our $page = $input_params{'page'};
> if (defined $page) {
> if ($page =~ m/[^0-9]/) {
Nice, almost trivial change.
> @@ -604,35 +646,12 @@ sub href (%) {
[...]
> if ($params{-replay}) {
> - while (my ($name, $symbol) = each %mapping) {
> + while (my ($name, $symbol) = each %cgi_param_mapping) {
> if (!exists $params{$name}) {
> - # to allow for multivalued params we use arrayref form
> - $params{$name} = [ $cgi->param($symbol) ];
> + $params{$name} = $input_params{$name};
> }
> }
> }
Nice cleanup.
> @@ -672,6 +691,25 @@ sub href (%) {
> ## ======================================================================
> ## validation, quoting/unquoting and escaping
>
> +sub validate_action {
> + my $input = shift || return undef;
> + return undef unless exists $actions{$action};
> + return $action;
> +}
> +
> +sub validate_project {
> + my $input = shift || return undef;
> + if (!validate_pathname($input) ||
> + !(-d "$projectroot/$input") ||
> + !check_head_link("$projectroot/$input") ||
> + ($export_ok && !(-e "$projectroot/$input/$export_ok")) ||
> + ($strict_export && !project_in_list($input))) {
> + return undef;
> + } else {
> + return $input;
> + }
> +}
Nice.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: files missing after converting a cvs repository to git
From: Michael Haggerty @ 2008-10-07 10:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <gcfcrg$ubc$1@ger.gmane.org>
Jakub Narebski wrote:
> Andreas Ericsson wrote:
>
>> cvs2svn is apparently quite good at getting even the weirdest history
>> right. Perhaps you can try that and then running "git svn" on the
>> resulting svn repo?
>
> cvs2svn has git output now (I think it is actually fast-import output)
Correct. See http://cvs2svn.tigris.org/cvs2git.html for more info. I
suggest using the trunk version of cvs2svn for conversions to git.
Please note, however, that cvs2svn can only handle one-time conversions
(i.e., not tracking a live CVS repository incrementally).
Michael
^ permalink raw reply
* Re: files missing after converting a cvs repository to git
From: Jakub Narebski @ 2008-10-07 10:15 UTC (permalink / raw)
To: git
In-Reply-To: <48EAFCEF.8030907@op5.se>
Andreas Ericsson wrote:
> cvs2svn is apparently quite good at getting even the weirdest history
> right. Perhaps you can try that and then running "git svn" on the
> resulting svn repo?
cvs2svn has git output now (I think it is actually fast-import output)
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH RFC] rebase--interactive: if preserving merges, use first-parent to limit what is shown.
From: Avi Kivity @ 2008-10-07 9:57 UTC (permalink / raw)
To: Stephen Haberman; +Cc: git
In-Reply-To: <20081006212021.04ba9214.stephen@exigencecorp.com>
Stephen Haberman wrote:
> However, t3404 makes a good point that if the right hand of the merge
> has parents that are going to get rebased, the right hand side does
> need to be included/shown/rewritten.
>
>
But, won't those commits get linearized? Won't git rebase pick the
commits into the left-hand side of the merge instead of into the right
hand side?
If git rebase is to handle nonlinear history, it needs much more
expressive commands; not only saying which commit to pick, but also what
the commit's parents shall be.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [msysGit] Re: [FYI][PATCH] Customizing the WinGit installer
From: Petr Baudis @ 2008-10-07 9:28 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry Potapov, Johannes Schindelin, Jakub Narebski, msysgit, git
In-Reply-To: <alpine.LFD.2.00.0810061909260.3208@nehalem.linux-foundation.org>
On Mon, Oct 06, 2008 at 07:10:37PM -0700, Linus Torvalds wrote:
> On Tue, 7 Oct 2008, Dmitry Potapov wrote:
> >
> > It looks like you have changed your opinion since what you wrote half
> > a year ago.
>
> Back then, nobody had really complained and sent in a patch to make it
> optional.
>
> That changes things. Once some user actually complains, and sends in a
> fix to make the whole dialog optional, I don't see why anybody would ever
> argue against such a patch being accepted.
Note that as mentioned in the original mail, my patch was not meant
for application upstream, just as an example for others who would like
to customize the Git installer for a particular environment. We wanted
to make Git installation/usage as simple as possible, reducing any
unnecessary steps we could - and this was an easy one.
I don't think the idea of showing GPL during installation makes
a lot of sense on its own, but I don't care much either and clicking
through licences and EULAs in wizards is a fact of life on Windows,
sadly. At least it doesn't require you to scroll down through the
licence as IIRC OpenOffice or GIMP or a similar beast did.
--
Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Heikki Orsila @ 2008-10-07 7:46 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Linus Torvalds, Johannes Schindelin, Jakub Narebski, Petr Baudis,
msysgit, git
In-Reply-To: <20081007030824.GW21650@dpotapov.dyndns.org>
On Tue, Oct 07, 2008 at 07:08:24AM +0400, Dmitry Potapov wrote:
> On Mon, Oct 06, 2008 at 07:10:37PM -0700, Linus Torvalds wrote:
> >
> > That changes things. Once some user actually complains, and sends in a
> > fix to make the whole dialog optional, I don't see why anybody would ever
> > argue against such a patch being accepted.
>
> First, he did not complain. He did not even mention that in the commit
> message.
What does that matter? People are complaining now.
> He mentioning some other things like removing release notes,
> but not the license. Second, I would expect that any change that goes
> against the previous achieved agreement may deserve some discussion,
> and not blindly accepted just because the user sent a patch.
Are you thinking this is some kind of decision by committee
organization? NO. Patches are picked up by maintainers. Anyone can
comment on the patches as they are published.
> Okay, I
> don't really care whether the installer shows the license or not...
Then why argue about it? I care very much.
--
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Heikki Orsila @ 2008-10-07 7:42 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Johannes Schindelin, Linus Torvalds, Jakub Narebski, Petr Baudis,
msysgit, git
In-Reply-To: <20081007015942.GV21650@dpotapov.dyndns.org>
On Tue, Oct 07, 2008 at 05:59:42AM +0400, Dmitry Potapov wrote:
> On Mon, Oct 06, 2008 at 11:11:25PM +0300, Heikki Orsila wrote:
> >
> > I disagree VIOLENTLY with you. I've been utterly struck with this
> > Windows crap here.. I spent a day installing stupid trivial software
> > and answering pointless EULAs. I REALLY REALLY hate extra questions..
>
> Git installer should NOT ask you whether you agree with the license,
> but it merely shows you the license. (At least, it is what was decided
> half a year ago).
It shouldn't even show that. Also, decisions can be changed..
> > This is my 20th reboot..
>
> I am pretty sure that 20th reboot has nothing to do with how many times
> the license has been shown...
Well, I've clicked through annoying "I accept" EULAs at least that many
times already..
--
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd
^ permalink raw reply
* Re: [StGit PATCH 5/6] Refresh the main stg man page
From: Karl Hasselström @ 2008-10-07 7:40 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0810061414ja87488k6aef65fec0856144@mail.gmail.com>
On 2008-10-06 22:14:05 +0100, Catalin Marinas wrote:
> 2008/10/5 Karl Hasselström <kha@treskal.com>:
>
> > --- a/Documentation/stg.txt
> > +++ b/Documentation/stg.txt
> [...]
> > + * After making changes to the worktree, you can incorporate the
> > + changes into an existing patch; this is called 'refreshing'. You
> > + may refresh any patch, not just the topmost one.
>
> I wouldn't advertise the refreshing of "any" patch as it doesn't
> always work (it actually fails in a lot of cases). Or at least we
> could mention that there are some restrictions.
Well, when it does fail, it fails in a controlled way, and lets the
user manually sort out the conflicts or undo. But I guess we could say
that.
When I wrote this, I didn't yet have a "Conflicts" chapter to point
to. :-)
> > + * You can easily 'rebase' your patch stack on top of any other Git
> > + branch.
>
> It might be better with something like "on top of a different Git
> commit". The first thought when reading the above is that you can
> move the patch stack to a different Git branch easily, which is not
> the case (you need to cherry-pick the patches).
Right, that makes sense. (We might want to mention "remote branch"
explicitly, though, since that's by far the most common case.)
> > + * The patch stack is just some extra metadata attached to regular
> > + Git commits, so you can continue to use Git tools along with
> > + StGit.
>
> Again, this is with some restrictions (or there aren't any with the
> new infrastructure?).
Well, "stg undo" can repair any havoc the git tools can make. And by
"havoc" I mean merge or rebase.
This should probably grow a short disclaimer, and point to the "git
interoperability" chapter.
> > + Tracking changes from a remote branch, while maintaining local
> > + modifications against that branch, possibly with the intent of
> > + sending some patches upstream. You can modify your patch stack as
> > + much as you want, and when your patches are finally accepted
> > + upstream, the permanent recorded Git history will contain just the
> > + final sequence of patches, and not the messy sequence of edits that
> > + produced them.
>
> Maybe we could mention that the local history is also clean, not
> only the upstream tree (though you mention it later in a different
> hunk).
Hmm, yes. Though I can't immediately think of any wording that doesn't
make that paragraph a lot more complicated. Suggestions welcome.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* [QGit bug] git user settings not retrieved when launched for Windows explorer
From: Abdelrazak Younes @ 2008-10-07 7:33 UTC (permalink / raw)
To: Git Mailing List, Marco Costalba
Dear Marco,
When I double click on qgit.exe, the user name and email are not shown
in the user settings (for any of the 3 combo values). But if I run qgit
from the commandline at the mysysgit bash prompt, the boxes are properly
filled. I would like to debug it but, as I reported last week, the MSVC
project doesn't work for me.
By the way, these two edit boxes are not editable on Windows, is that on
purpose? If yes, maybe we could let the user change them is 'Local
config' is selected and call the appropriate git function?
Thanks,
Abdel.
^ permalink raw reply
* Re: [StGit PATCH 5/6] Refresh the main stg man page
From: Karl Hasselström @ 2008-10-07 7:32 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0810061415n551acbc5r812bbd0e75eeb572@mail.gmail.com>
On 2008-10-06 22:15:42 +0100, Catalin Marinas wrote:
> 2008/10/5 Karl Hasselström <kha@treskal.com>:
>
> > + * You can easily 'rebase' your patch stack on top of any other Git
> > + branch. (The 'base' of a patch stack is the most recent Git commit
> > + that is not an StGit patch.) For example, if you started making
>
> You may want to move the bracket before the dot.
You mean like this?
You can easily 'rebase' your patch stack on top of any other Git
branch (the 'base' of a patch stack is the most recent Git commit
that is not an StGit patch).
I slightly prefer the original, since the paretheses contain a full
stand-alone sentence, but it doesn't matter that much.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [StGit PATCH 6/6] Refresh and expand the tutorial (not finished)
From: Karl Hasselström @ 2008-10-07 7:12 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0810061425u3ead1de5re1a252f03c2d09b3@mail.gmail.com>
On 2008-10-06 22:25:45 +0100, Catalin Marinas wrote:
> 2008/10/5 Karl Hasselström <kha@treskal.com>:
>
> > This is a first pass at expanding the tutorial, fixing its
> > formatting, and updating it with the new things that have happened
> > in StGit.
> >
> > There are a number of things still left to do in the second half
> > of the document; they are tagged with "TODO".
>
> Thanks for this. Even with the TODOs, I think we can merge them into
> the master branch so that I have the same copy as you.
OK, that makes sense; "master" is, after all, a development branch. A
stable development branch, but still a development branch.
> Do you plan to do more work on the tutorial (so that we don't
> duplicate)?
I was planning to finish it if nobody else stepped up. But it'd be
great if I didn't have to do it alone. :-)
I guess we can avoid duplicate work if we send out a mail saying "I'll
now be working on part XXX" before we start writing.
> > -Layout of the .git directory
>
> Should we mention the metadata storage somewhere? This is probably
> too advanced for the tutorial but might be useful to have it in a
> development document (for other to understand where we keep things).
Yes, probably.
> Actually, is the automatically generated documentation from the new
> infrastructure enough?
I doubt it. It probably could be made enough, though, but I don't know
if that'd be better or worse than a separate document -- the code
documentation is tightly tied to the code, and I'm not sure if we want
that in this case.
We can let whoever writes it decide.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [StGit PATCH 4/6] Add 1.0 TODO items from recent discussion by private mail
From: Karl Hasselström @ 2008-10-07 7:07 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0810061350k6b90328cqe49db4b0d3f6c87e@mail.gmail.com>
On 2008-10-06 21:50:32 +0100, Catalin Marinas wrote:
> 2008/10/5 Karl Hasselström <kha@treskal.com>:
>
> > +- Convert the remaining commands to the new infrastructure.
> > +
> > +- Go through the design of the UI and make sure there's nothing hard
> > + to change in there that we'll regret later.
> > +
> > +- Write a user guide. I'm thinking a document on the order of 10-30
> > + pages that'll explain why one would want to use StGit, and how.
> > +
> > +- Make sure the rest of the documentation is in good shape.
>
> Actually, at least for these kind of things we should still keep
> this file. We could drop it after 1.0.
Hmm, OK. Though the issue tracker would probably handle this kind of
stuff too -- doesn't it have a way to tag issues with the desired
milestone, and then present a view with, for example, all the
remaining "1.0" issues?
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [StGit PATCH 2/6] Remove TODO items that have already been addressed
From: Karl Hasselström @ 2008-10-07 7:05 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0810061343g3f675356x960aff02e519bb36@mail.gmail.com>
On 2008-10-06 21:43:47 +0100, Catalin Marinas wrote:
> 2008/10/5 Karl Hasselström <kha@treskal.com>:
>
> > TODO | 9 ---------
> > 1 files changed, 0 insertions(+), 9 deletions(-)
>
> We could also drop this file entirely since we have a Tasks list on
> the gna.org project page.
Right, that's probably a good idea.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH RFC] rebase--interactive: if preserving merges, use first-parent to limit what is shown.
From: Stephen Haberman @ 2008-10-07 6:36 UTC (permalink / raw)
To: Avi Kivity; +Cc: git
In-Reply-To: <20081006212021.04ba9214.stephen@exigencecorp.com>
> So, unless I think of something else, I'm done hacking on this and am
> withdrawing the patch.
>
> Though I am curious--with the sequencer, is the Avi/my request of not
> listing out-of-band commits in the todo file going to be handled?
>
> Some sort of "--first-parent-unless-included-in-rebase" flag.
Okay, I lied--I have a patch that implements this behavior, passes Avi's
script-turned-test, and passes t3404. It implements the above algorithm
of, if in preserving merges mode, start with only first parents in the
todo list, and then recursively prepend right hand side commits to the
todo list only if their parents are going to be rewritten. This drops a
lot of cruft from rebase-i-p with large merges and is very cool, IMHO.
However, lest I burn my "PATCH v2" opportunity, I'm holding off on
posting the updated patch. It works and passes tests but I'm sure I'll
tinker with it some more over the next few days. It will also likely
conflict with my pu sh/maint-rebase3 patch, so I don't know whether to
base it on top of that one or not (guessing not).
Also, I think the patch itself is less interesting than the discussion
of whether this "first parent only" behavior is desired or not.
Obviously I think so--do others agree/disagree?
I've read more into the sequencer, and from what I can tell it still
just drives off a todo of pick/etc. input, and does not generate the
todo itself. So I think my patch is still fair game in terms of how to
generate either the current or the next generation rebase-i-p todo list.
I could be wrong on that though.
Thanks,
Stephen
^ permalink raw reply
* Re: [PATCH v2] correct verify_path for Windows
From: Alex Riesen @ 2008-10-07 6:25 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Joshua Juran, Giovanni Funchal, git, Shawn O. Pearce,
Johannes Sixt
In-Reply-To: <20081007032623.GX21650@dpotapov.dyndns.org>
2008/10/7 Dmitry Potapov <dpotapov@gmail.com>:
> +#if defined(_WIN32) || defined(__CYGWIN__)
> + /* On Windows, file names are case-insensitive */
> + case 'G':
> + if ((rest[1]|0x20) != 'i')
> + break;
> + if ((rest[2]|0x20) != 't')
> + break;
> +#else
Maybe it is already time for FILESYSTEM_CASEINSENSITIVE?
^ permalink raw reply
* Re: [PATCH] Implement git clone -v
From: Alex Riesen @ 2008-10-07 6:21 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Constantine Plotnikov, git
In-Reply-To: <1223331590-22138-1-git-send-email-vmiklos@frugalware.org>
2008/10/7 Miklos Vajna <vmiklos@frugalware.org>:
> The new -v option forces the progressbar, even in case the output is not
> a terminal.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Sat, Oct 04, 2008 at 11:42:15PM +0200, Alex Riesen <raa.lkml@gmail.com> wrote:
>> 2008/10/3 Constantine Plotnikov <constantine.plotnikov@gmail.com>:
>> > Is there a way to force a progress output on stderr for git clone
>> > preferably using options or environment variables?
>>
>> No, but "-v" option is not used for anything yet, so you are free to
>> implement it.
>
> Something like this?
>
Yes. Does it work? :)
^ permalink raw reply
* Re: [PATCH 0/4] diff text conversion filter
From: Matthieu Moy @ 2008-10-07 6:15 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git
In-Reply-To: <20081007060014.GA7965@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Oct 07, 2008 at 07:52:02AM +0200, Johannes Sixt wrote:
>
>> How about this: If I run 'git show -- foo.doc' (foo.doc resolves to a
>> single path, obviously), I want MS Word, but for other uses of 'git show'
>> I don't. I think that heuristics could be very effective: With a plain
>> 'git show' I get the overview of the change, and with 'git show --
>> foo.doc' I drill down into a single document.
>
> Hrm. I am not opposed to heuristics, but in this case, I don't like the
> one you have proposed. ;P
>
> My specific case that prompted this work is a repository full of
> pictures and videos, where I rarely (if ever) change the media content,
> but frequently change exif tags. So my "usual" case is to want to see
> "git log -p" with the textconv'd version. The commit diffs are otherwise
> totally meaningless.
So, you disagree about "git log" not showing the textconv, but you
still agree with half of the proposal :-P. When the user explicitely
requests a single file, he does want textconv (requesting a diff for a
single file and be happy with "binary files differ" would be
strange ...).
It seems quite clear to me that we won't get a heuristic right for
everybody (some diff driver are fast, some are slow, some require an
external GUI, some don't, ...). Better let the thing be nicely
configurable IMHO.
One proposal: have a diff.<driver>.activate with several values:
* "always": activate the diff driver in any porcelain
* "diff": activate it only for "git diff", as currently
* "singlefile": Johannes's heuristic proposal
That way, one could say easily "activate exiftags filter all the time,
but MS-Word only when I request a diff for a single file", and this
leaves room for other values if the need be. Well, there's no room for
"use MS-Word native diff tool in git-gui but antiword/catdoc +
textconv in 'git log -p'" here, but do we want it?
Or is all that just overkill?
--
Matthieu
^ permalink raw reply
* Re: [PATCH v2] correct verify_path for Windows
From: Johannes Sixt @ 2008-10-07 6:18 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Joshua Juran, Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <20081007032623.GX21650@dpotapov.dyndns.org>
Dmitry Potapov schrieb:
> +#if defined(_WIN32) || defined(__CYGWIN__)
I think that for consistency you should use __MINGW32__ instead of _WIN32.
> + /* On Windows, file names are case-insensitive */
> + case 'G':
> + if ((rest[1]|0x20) != 'i')
> + break;
> + if ((rest[2]|0x20) != 't')
> + break;
We have tolower().
-- Hannes
^ permalink raw reply
* Re: Files with colons under Cygwin
From: Johannes Sixt @ 2008-10-07 6:13 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <20081007005327.GT21650@dpotapov.dyndns.org>
Dmitry Potapov schrieb:
> On Mon, Oct 06, 2008 at 08:54:44AM +0200, Johannes Sixt wrote:
>> [*] I say "meaningful" and not "necessary" because the situation is just
>> like when you grab some random SoftwarePackage.tar.gz, and run ./configure
>> without looking first what it is going to do.
>
> When I grab any tar, I can look at its context without myself of any
> risk that some files can be overwritten on my file system. And when
> I want to look at some remote git repository, I usually do:
>
> git clone URL
>
> If it can overwrite some files behind my back, it is security a hole.
Fair enough.
> On Linux (or other sane file systems), we have all required checks to
> prevent that from happening, and they are places in verify_path, which
> prevents malicious names entering into the index and thus to the file
> system too. So, we should do all required checks on Windows too.
I don't object the intention of your patch. But I cannot judge whether
verify_path() is the correct location to put the checks because I don't
know this part of the code. I leave the final word to others.
-- Hannes
^ permalink raw reply
* Re: files missing after converting a cvs repository to git
From: Andreas Ericsson @ 2008-10-07 6:08 UTC (permalink / raw)
To: Adam Mercer; +Cc: GIT
In-Reply-To: <799406d60810061502y417ec53o1a1f5cef800dfe45@mail.gmail.com>
Adam Mercer wrote:
> Hi
>
> One of the prrojects I am involved with is currently looking into
> migrating from cvs to git, therefore we have been investigating this
> by setting up a git repository that tracks cvs, however there are some
> very strange things going on and I was hoping someone could offer some
> insight into what is going on.
>
> I use the following git cvsimport command to import the repository:
>
> $ git cvsimport -v -a -i -k
> -d:pserver:user@server:port/path/to/cvs/repo -C /path/to/new/git/repo
> module
>
> this ran successfully with no warnings or errors. However, when I
> checkout the new git repository that are several files missing from
> the git checkout that are present in the cvs checkout.
>
> Does anyone know why this would happen, or how to find out?
Has the CVS repo been tampered with in the past? If so, it's entirely
possible that checking out and working with CVS works just fine, but
getting history into coherent changesets is impossible.
Can you checkout the offending (old) revisions using CVS? If so, there
is indeed a bug in the cvs-importer. Otherwise, your CVS repo is hosed
and you're reduced to running whatever conversion tool does the best
job if you want to maintain history.
cvs2svn is apparently quite good at getting even the weirdest history
right. Perhaps you can try that and then running "git svn" on the
resulting svn repo?
Good luck. You'll probably need it :-/
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] error out if path is invalid
From: Johannes Sixt @ 2008-10-07 6:03 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Shawn O. Pearce, git
In-Reply-To: <20081007002221.GS21650@dpotapov.dyndns.org>
Dmitry Potapov schrieb:
> On Mon, Oct 06, 2008 at 09:02:22AM +0200, Johannes Sixt wrote:
>> Dmitry Potapov schrieb:
>>> if (!verify_path(path))
>>> - return -1;
>>> + return error("Invalid path '%s'", path);
>> Look at this change. Didn't the code error out before, too?
>
> It is certainly did not here. As to its caller, it depends. In fact,
> there are two chunks like that in my patch, so I am not sure to which
> one you refer here. If we speak about add_cacheinfo() then though the
> function did not error out, its caller died with one of the following
> messages:
> git update-index: unable to update some-file-name
> or
> git update-index: --cacheinfo cannot add some-file-name
Look at the original patch. You did not change the behavior except to
write more error messages. Maybe I misunderstand the words "to error out".
I understand them as "to detect an error and return early", but not "write
an error message".
> However, if we speak about add_index_entry_with_check then the caller
> will not produce any error. The git would exit successfully (it still
> does) and there was no error message as if everything was fine.
>
> Perhaps, the exit code should be corrected too, but if the git just dies
> when add_index_entry() fails it may cause that having one invalid path
> will prevent to check out other files, which does not seem to be the
> right thing to do.
>
> As to correction to correction to make_cache_entry then after my
> previous patch, it started to error out:
>
> make_cache_entry failed for path 'some-file-name'
>
> before that it silently segfaulted.
>
>> Same in the
>> other cases. Hence, your patch subject does not describe the patch.
>
> Should I include the above explanation in the commit message or do you
> have any objection to having the above error message in cases where the
> caller already produce some message when it dies?
I don't object the change, only its (missing or IMHO incorrect)
justification. I don't think that the above text would be the correct
description because as far as I can see the only change you made was to
add error messages.
-- Hannes
^ permalink raw reply
* Re: [PATCH 0/4] diff text conversion filter
From: Jeff King @ 2008-10-07 6:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Matthieu Moy, git
In-Reply-To: <48EAF902.3040402@viscovery.net>
On Tue, Oct 07, 2008 at 07:52:02AM +0200, Johannes Sixt wrote:
> How about this: If I run 'git show -- foo.doc' (foo.doc resolves to a
> single path, obviously), I want MS Word, but for other uses of 'git show'
> I don't. I think that heuristics could be very effective: With a plain
> 'git show' I get the overview of the change, and with 'git show --
> foo.doc' I drill down into a single document.
Hrm. I am not opposed to heuristics, but in this case, I don't like the
one you have proposed. ;P
My specific case that prompted this work is a repository full of
pictures and videos, where I rarely (if ever) change the media content,
but frequently change exif tags. So my "usual" case is to want to see
"git log -p" with the textconv'd version. The commit diffs are otherwise
totally meaningless.
But I think for many others, they are primarily working with text data,
but have some (for example) binary word processor documents. Triggering
such conversions for a single file might make more sense there.
So I'm not sure there is a heuristic that serves both desires. Which is
why I would lean towards a command-line argument, backed by a config
option for those repositories which really want it all the time (and let
me reiterate that such a config option would still have _no_ impact on
plumbing; applying text conversion there is just plain wrong).
> Or this: 'git show -p' uses the textconv'd version, 'git show' does not
> ("Binary files differ").
At that point, is there really an advantage over "git show --textconv"?
> BTW, also with 'git diff' I sometimes don't want MS Word to pop up...
Yes. It is annoying that git can't simply read our minds. :)
-Peff
^ permalink raw reply
* Re: [PATCH 0/4] diff text conversion filter
From: Johannes Sixt @ 2008-10-07 5:52 UTC (permalink / raw)
To: Jeff King; +Cc: Matthieu Moy, git
In-Reply-To: <20081007012044.GA4217@coredump.intra.peff.net>
Jeff King schrieb:
> On Mon, Oct 06, 2008 at 05:15:22PM +0200, Matthieu Moy wrote:
>
>> Actually, I understand you don't want git gui and gitk to load MS-Word
>> anytime you click something, but I'd love to see the textconv+diff in
>> gitk.
>>
>> (yeah, that's pretty hard to specify right, the ideal requirement
>> seems to be "in a gui, use the good part of the diff driver, but not
>> the other" :-\).
>
> I think it is even more complex than that. Sometimes when doing "git
> show" I want to see the textconv'd version, and sometimes I don't. So I
> really want a command-line flag or environment variable that I can use
> to control it (with a sane default).
How about this: If I run 'git show -- foo.doc' (foo.doc resolves to a
single path, obviously), I want MS Word, but for other uses of 'git show'
I don't. I think that heuristics could be very effective: With a plain
'git show' I get the overview of the change, and with 'git show --
foo.doc' I drill down into a single document.
Or this: 'git show -p' uses the textconv'd version, 'git show' does not
("Binary files differ").
BTW, also with 'git diff' I sometimes don't want MS Word to pop up...
-- Hannes
^ permalink raw reply
* Re: [PATCH v2] git init: --bare/--shared overrides system/global config
From: Deskin Miller @ 2008-10-07 5:37 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081006141452.GA7684@spearce.org>
If core.bare or core.sharedRepository are set in /etc/gitconfig or
~/.gitconfig, then 'git init' will read the values when constructing a
new config file; reading them, however, will override the values
specified on the command line. In the case of --bare, this ends up
causing a segfault, without the repository being properly initialised;
in the case of --shared, the permissions are set according to the
existing config settings, not what was specified on the command line.
This fix saves any specified values for --bare and --shared prior to
reading existing config settings, and restores them after reading but
before writing the new config file. core.bare is ignored in all
situations, while core.sharedRepository will only be used if --shared
is not specified to git init.
Also includes testcases which use a specified global config file
override, demonstrating the former failure scenario.
Signed-off-by: Deskin Miller <deskinm@umich.edu>
---
On Mon, Oct 06, 2008 at 07:14:52AM -0700, Shawn O. Pearce wrote:
> Deskin Miller <deskinm@umich.edu> wrote:
> > From b6144562983703079a1eba8cdac3506c18d751a3 Mon Sep 17 00:00:00 2001
> > From: Deskin Miller <deskinm@umich.edu>
> > Date: Sat, 4 Oct 2008 20:07:44 -0400
>
> FWIW please don't include these lines in the commit message part
> of the patch email. [...]
Alright, sorry for the messiness; I guess I thought preserving the original
commit date was important? Won't happen again.
> > diff --git a/builtin-init-db.c b/builtin-init-db.c
> > index 8140c12..38e282c 100644
> > --- a/builtin-init-db.c
> > +++ b/builtin-init-db.c
> > @@ -191,6 +194,8 @@ static int create_default_files(const char *template_path)
> > copy_templates(template_path);
> >
> > git_config(git_default_config, NULL);
> > + is_bare_repository_cfg = init_is_bare_repository;
> > + shared_repository = init_shared_repository;
>
> Is this really the right thing to do? It seems like it would prevent
> a user from setting core.sharedRepository = group in their template
> and thus always have a shared repository on their system.
You're right. Fixed in this version: core.bare ignores any config, while
--shared will override config settings, but init will use the config settings
if --shared is not specified.
> A second related test would be a ~/.gitconfig which sets
> core.sharedRepository = 0666 and then does "git init". I think
> the right outcome is a repository which has that set.
Good suggestion, I added such a case in this version. My first version fails
this new testcase, while maint fails the original testcase I wrote.
builtin-init-db.c | 12 ++++++++++--
t/t0001-init.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 8140c12..d30c3fe 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -17,6 +17,9 @@
#define TEST_FILEMODE 1
#endif
+static int init_is_bare_repository = 0;
+static int init_shared_repository = -1;
+
static void safe_create_dir(const char *dir, int share)
{
if (mkdir(dir, 0777) < 0) {
@@ -191,6 +194,9 @@ static int create_default_files(const char *template_path)
copy_templates(template_path);
git_config(git_default_config, NULL);
+ is_bare_repository_cfg = init_is_bare_repository;
+ if (init_shared_repository != -1)
+ shared_repository = init_shared_repository;
/*
* We would have created the above under user's umask -- under
@@ -277,6 +283,8 @@ int init_db(const char *template_dir, unsigned int flags)
safe_create_dir(get_git_dir(), 0);
+ init_is_bare_repository = is_bare_repository();
+
/* Check to see if the repository version is right.
* Note that a newly created repository does not have
* config file, so this will not fail. What we are catching
@@ -381,9 +389,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
sizeof(git_dir)), 0);
} else if (!strcmp(arg, "--shared"))
- shared_repository = PERM_GROUP;
+ init_shared_repository = PERM_GROUP;
else if (!prefixcmp(arg, "--shared="))
- shared_repository = git_config_perm("arg", arg+9);
+ init_shared_repository = git_config_perm("arg", arg+9);
else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
flags |= INIT_DB_QUIET;
else
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 620da5b..5ac0a27 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -167,4 +167,36 @@ test_expect_success 'init with --template (blank)' '
! test -f template-blank/.git/info/exclude
'
+test_expect_success 'init --bare/--shared overrides system/global config' '
+ (
+ HOME="`pwd`" &&
+ export HOME &&
+ test_config="$HOME"/.gitconfig &&
+ unset GIT_CONFIG_NOGLOBAL &&
+ git config -f "$test_config" core.bare false &&
+ git config -f "$test_config" core.sharedRepository 0640 &&
+ mkdir init-bare-shared-override &&
+ cd init-bare-shared-override &&
+ git init --bare --shared=0666
+ ) &&
+ check_config init-bare-shared-override true unset &&
+ test x0666 = \
+ x`git config -f init-bare-shared-override/config core.sharedRepository`
+'
+
+test_expect_success 'init honors global core.sharedRepository' '
+ (
+ HOME="`pwd`" &&
+ export HOME &&
+ test_config="$HOME"/.gitconfig &&
+ unset GIT_CONFIG_NOGLOBAL &&
+ git config -f "$test_config" core.sharedRepository 0666 &&
+ mkdir shared-honor-global &&
+ cd shared-honor-global &&
+ git init
+ ) &&
+ test x0666 = \
+ x`git config -f shared-honor-global/.git/config core.sharedRepository`
+'
+
test_done
--
1.6.0.2.307.gc427
^ 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