* Re: Feature Request - Hide ignored files before checkout
From: Andrew Ardill @ 2012-12-10 1:46 UTC (permalink / raw)
To: Matthew Ciancio; +Cc: git@vger.kernel.org
In-Reply-To: <000301cdd4dd$f8554090$e8ffc1b0$@gmail.com>
Hi Matt,
On 8 December 2012 11:50, Matthew Ciancio <matthew.ciancio16@gmail.com> wrote:
> Problem: ignore.txt does not "disappear" like foo.txt does and is now just
> sitting in branchA (and now any other branch I checkout into).
>
> When I first started using Git, I genuinely thought this was a bug, because
> it seems so logical to me that ignore files should hide/reappear just like
> tracked files do, when switching branches.
Let me address this by asking a few questions; *why* do files
hide/reappear, what is the mechanism behind that and does it really
make sense to apply it to ignored files.
For each commit, git stores a snapshot of your files. When we switch
branches we are telling git to restore the previously saved snapshot
so we can work with those files. This means resetting the working
directory so that it looks like what we had committed; git will delete
files that were part of the current checked out snapshot but not the
new one, and create files that need to be created. As a convenience to
users, files that are not tracked are left 'as-is' when switching
branches.
So we see that in order to hide/reappear a file it has to be tracked
in a snapshot, and so has to be committed *somewhere*. An ignored file
is by definition not included in commits, and furthermore you hope to
keep these files out of your commit history.
> I have been told ways of circumventing this (using commits and un-commits OR
> using stash), but my reason for avoiding commits is: say you have binary/OS
> specific files which really do not belong in the commit logs (even locally)
> and hence should be ignored. What if you want those files in only one branch
> and not all?
> Stashing doesn't seem appropriate either, because it would get messy.
I am not sure how viable a suggestion this is, but perhaps you can
have two separate repositories, one tracking your standard branches,
and another tracking the ignored files. These repositories could be
kept in sync through submodules or some similar mechanism. This could
also allow you to, for example, publish the histories of these
independently, for example releasing the non-ignored repository
publicly.
I haven't heard of anyone doing this, but if you need to keep the
history clean it might be a way of achieving it.
I also don't know what the implications of checking out two
repositories into the same tree might be, or even if git would allow
it in general (maybe if you ignored everything belonging to the other
repository?) In any case, this solution could quickly become messy,
but if carefully controlled might solve your problem. Then again,
maybe you can achieve what you want using more 'traditional' git
workflows.
Regards,
Andrew Ardill
^ permalink raw reply
* RE: Feature Request - Hide ignored files before checkout
From: Matthew Ciancio @ 2012-12-10 2:01 UTC (permalink / raw)
To: 'Andrew Ardill'; +Cc: git
In-Reply-To: <CAH5451m-JcgLtvVER1UgvsFzemb=otG3XttR4j2s=eFnPrPyEQ@mail.gmail.com>
Thanks for explaining that Andrew. I guess that was my intention: to have an "ignored file snapshot", but I can see now that it goes against Git's definitions and is not really needed.
I have overcome the problem by re-organising my repository and "... using more 'traditional' git workflows.".
-----Original Message-----
From: Andrew Ardill [mailto:andrew.ardill@gmail.com]
Sent: Monday, 10 December 2012 12:46 PM
To: Matthew Ciancio
Cc: git@vger.kernel.org
Subject: Re: Feature Request - Hide ignored files before checkout
Hi Matt,
On 8 December 2012 11:50, Matthew Ciancio <matthew.ciancio16@gmail.com> wrote:
> Problem: ignore.txt does not "disappear" like foo.txt does and is now
> just sitting in branchA (and now any other branch I checkout into).
>
> When I first started using Git, I genuinely thought this was a bug,
> because it seems so logical to me that ignore files should
> hide/reappear just like tracked files do, when switching branches.
Let me address this by asking a few questions; *why* do files hide/reappear, what is the mechanism behind that and does it really make sense to apply it to ignored files.
For each commit, git stores a snapshot of your files. When we switch branches we are telling git to restore the previously saved snapshot so we can work with those files. This means resetting the working directory so that it looks like what we had committed; git will delete files that were part of the current checked out snapshot but not the new one, and create files that need to be created. As a convenience to users, files that are not tracked are left 'as-is' when switching branches.
So we see that in order to hide/reappear a file it has to be tracked in a snapshot, and so has to be committed *somewhere*. An ignored file is by definition not included in commits, and furthermore you hope to keep these files out of your commit history.
> I have been told ways of circumventing this (using commits and
> un-commits OR using stash), but my reason for avoiding commits is: say
> you have binary/OS specific files which really do not belong in the
> commit logs (even locally) and hence should be ignored. What if you
> want those files in only one branch and not all?
> Stashing doesn't seem appropriate either, because it would get messy.
I am not sure how viable a suggestion this is, but perhaps you can have two separate repositories, one tracking your standard branches, and another tracking the ignored files. These repositories could be kept in sync through submodules or some similar mechanism. This could also allow you to, for example, publish the histories of these independently, for example releasing the non-ignored repository publicly.
I haven't heard of anyone doing this, but if you need to keep the history clean it might be a way of achieving it.
I also don't know what the implications of checking out two repositories into the same tree might be, or even if git would allow it in general (maybe if you ignored everything belonging to the other
repository?) In any case, this solution could quickly become messy, but if carefully controlled might solve your problem. Then again, maybe you can achieve what you want using more 'traditional' git workflows.
Regards,
Andrew Ardill
^ permalink raw reply
* [PATCH] gitweb: Sort projects with undefined ages last
From: Matthew Daley @ 2012-12-10 4:34 UTC (permalink / raw)
To: git; +Cc: Matthew Daley
Sorting gitweb's project list by age ('Last Change') currently shows
projects with undefined ages at the head of the list. This results in a
less useful result when there are a number of projects that are missing
or otherwise faulty and one is trying to see what projects have been
updated recently.
Fix by sorting these projects with undefined ages at the bottom of the
list when sorting by age.
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
I realize this might be a bit bikesheddy, but it does improve the listing
in the given use case. For an example of the problem, see ie.
http://git.kernel.org/?o=age or http://repo.or.cz/w?a=project_list;o=age .
I'm also not a Perl native, so any advice on making the patch good Perl is
appreciated.
gitweb/gitweb.perl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0f207f2..21da1b5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5541,7 +5541,9 @@ sub sort_projects_list {
if ($oi->{'type'} eq 'str') {
@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @$projlist;
} else {
- @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @$projlist;
+ @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}}
+ grep {defined $_->{$oi->{'key'}}} @$projlist;
+ push @projects, grep {!defined $_->{$oi->{'key'}}} @$projlist;
}
return @projects;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] gitweb: Sort projects with undefined ages last
From: Junio C Hamano @ 2012-12-10 6:16 UTC (permalink / raw)
To: Matthew Daley; +Cc: git
In-Reply-To: <1355114061-4652-1-git-send-email-mattjd@gmail.com>
Matthew Daley <mattjd@gmail.com> writes:
> Sorting gitweb's project list by age ('Last Change') currently shows
> projects with undefined ages at the head of the list. This results in a
> less useful result when there are a number of projects that are missing
> or otherwise faulty and one is trying to see what projects have been
> updated recently.
>
> Fix by sorting these projects with undefined ages at the bottom of the
> list when sorting by age.
>
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---
> I realize this might be a bit bikesheddy, but it does improve the listing
> in the given use case. For an example of the problem, see ie.
> http://git.kernel.org/?o=age or http://repo.or.cz/w?a=project_list;o=age .
Yeah, it could be argued that in a very minor corner case showing
new and empty ones at the top might attract more attention to them,
but new and empty ones can stay inactive, so this change would be an
overall improvement for these two sites. An alternative could be to
give the mtime of the git directory to the age field if there is no
commits in the repository, to sink the empty and inactive ones to
the bottom quickly while showing newly created ones at the top, but
it shouldn't make any practical difference.
> I'm also not a Perl native, so any advice on making the patch good Perl is
> appreciated.
>
> gitweb/gitweb.perl | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0f207f2..21da1b5 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5541,7 +5541,9 @@ sub sort_projects_list {
> if ($oi->{'type'} eq 'str') {
> @projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @$projlist;
> } else {
> - @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @$projlist;
> + @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}}
> + grep {defined $_->{$oi->{'key'}}} @$projlist;
> + push @projects, grep {!defined $_->{$oi->{'key'}}} @$projlist;
> }
Two observations:
* This iterates over the same @$projlist twice with grep, with one
"defined" and the other "!defined", which may risk these two
complementary grep conditions to go out of sync (it also may
affect performance but that is a lessor issue).
An alternative may be to change the expression used inside sort()
to treat an undef as if it were a very large value, something
like:
sort {
defined $a->{$oi->{'key'}}
? (defined $b->{$oi->{'key'}}
? ($a->{$oi->{'key'}} <=> $b->{$oi->{'key'}})
: -1)
: (defined $b->{$oi->{'key'}} ? 1 : 0);
}
* This "sort undefs at the end is better than at the beginning" is
good only for the "age" field, and we wouldn't know if we would
add other keys for which it may be better to sort undef at the
beginning. The order_info{} currently has only one field of the
'num' type, so this is not an immediate issue, but in order to
future proof, it may make sense to rewrite the sort_projects_list
function to map the order field name to a function given to sort,
e.g.
my %order_sort = (
project => sub { $a->{'path'} cmp $b->{'path'} },
descr => sub { $a->{'descr_long'} cmp $b->{'descr_long'} },
owner => sub { $a->{'owner'} cmp $b->{'owner'} },
age => sub { ... the num cmp with undef above ... },
);
if (!exists $order_sort{$order}) {
return @$projlist;
}
return sort $order_sort{$order} @$projlist;
I am not sure the second one is worth it, though.
^ permalink raw reply
* Re: [PATCH] git(1): remove a defunct link to "list of authors"
From: Junio C Hamano @ 2012-12-10 6:31 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8A7AYpZs7mTc+B-F7BBLPdACim=gHCg8sK1Aci8YSEB4Q@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Sat, Dec 8, 2012 at 12:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> * If somebody has a working replacement URL, we could use that
>>> instead, of course. Takers?
>>
>> A possible alternative could be https://www.ohloh.net/p/git/contributors/summary
>
> Nice charts!
Yup.
Their numbers seem to be just 'any commit by the author, with
mailmap applied', and I am of two minds with it. Counting without
"shortlog --no-merges", depending on the management style of the
project, tends to credit the integrator too much. Even though
vetting the patches and choosing when to merge the topics is a
significant contribution, it isn't *that* big compared to the work
done by the contributor who took initiative to scratch that itch.
With or without "--no-merges", the big picture you can get out of
"git shortlog -s -n --since=1.year" does not change very much, but
the headline numbers give a wrong impression.
And of course, application of the mailmap is very important, if you
want to get meaningful numbers out of shortlog over a longer period.
^ permalink raw reply
* Re: [PATCH v2] cache-tree: invalidate i-t-a paths after generating trees
From: Junio C Hamano @ 2012-12-10 6:50 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <1354939803-8466-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/cache-tree.c b/cache-tree.c
> index 28ed657..989a7ff 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -248,6 +248,7 @@ static int update_one(struct cache_tree *it,
> int missing_ok = flags & WRITE_TREE_MISSING_OK;
> int dryrun = flags & WRITE_TREE_DRY_RUN;
> int i;
> + int to_invalidate = 0;
>
> if (0 <= it->entry_count && has_sha1_file(it->sha1))
> return it->entry_count;
> @@ -324,7 +325,13 @@ static int update_one(struct cache_tree *it,
> if (!sub)
> die("cache-tree.c: '%.*s' in '%s' not found",
> entlen, path + baselen, path);
> - i += sub->cache_tree->entry_count - 1;
> + i--; /* this entry is already counted in "sub" */
> + if (sub->cache_tree->entry_count < 0) {
> + i -= sub->cache_tree->entry_count;
> + to_invalidate = 1;
> + }
> + else
> + i += sub->cache_tree->entry_count;
Hrm. update_one() is prepared to see a cache-tree whose entry count
is zero (see the context lines in the previous hunk) and the
invariant for the rest of the code is "if 0 <= entry_count, the
cached tree is valid; invalid cache-tree has -1 in entry_count.
More importantly, entry_count negated does not in general express
how many entries there are in the subtree and does not tell us how
many index entries to skip.
> @@ -339,8 +346,23 @@ static int update_one(struct cache_tree *it,
> mode, sha1_to_hex(sha1), entlen+baselen, path);
> }
>
> - if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
> - continue; /* entry being removed or placeholder */
> + /*
> + * CE_REMOVE entries are removed before the index is
> + * written to disk. Skip them to remain consistent
> + * with the future on-disk index.
> + */
> + if (ce->ce_flags & CE_REMOVE)
> + continue;
> +
> + /*
> + * CE_INTENT_TO_ADD entries exist on on-disk index but
> + * they are not part of generated trees. Invalidate up
> + * to root to force cache-tree users to read elsewhere.
> + */
> + if (ce->ce_flags & CE_INTENT_TO_ADD) {
> + to_invalidate = 1;
> + continue;
> + }
Thanks for documenting these.
> @@ -360,7 +382,7 @@ static int update_one(struct cache_tree *it,
> }
>
> strbuf_release(&buffer);
> - it->entry_count = i;
> + it->entry_count = to_invalidate ? -i : i;
See above. I am not fundamentally opposed to a change to redefine
entry_count so that it always maintains how many index entries the
subtree covers, even for invalidated subtree, but I do not think
this patch alone is sufficient to maintain such invariant.
> #if DEBUG
> fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
> it->entry_count, it->subtree_nr,
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index ec35409..2a4a749 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -62,5 +62,25 @@ test_expect_success 'can "commit -a" with an i-t-a entry' '
> git commit -a -m all
> '
>
> +test_expect_success 'cache-tree invalidates i-t-a paths' '
> + git reset --hard &&
> + mkdir dir &&
> + : >dir/foo &&
> + git add dir/foo &&
> + git commit -m foo &&
> +
> + : >dir/bar &&
> + git add -N dir/bar &&
> + git diff --cached --name-only >actual &&
> + echo dir/bar >expect &&
> + test_cmp expect actual &&
> +
> + git write-tree >/dev/null &&
> +
> + git diff --cached --name-only >actual &&
> + echo dir/bar >expect &&
> + test_cmp expect actual
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-10 7:04 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: Soren Brinkmann, git
In-Reply-To: <CAKJhZwROXsTa4wu-C9rhfGysetL+cZRDECyFUn5VTb833pWzMQ@mail.gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
> Would like to get some more feedback on the proposed output in case of
> (1) an untracked subdirectory with multiple files where at least one of them
> cannot be removed.
> (2) reporting ignored untracked git subdirectories
>
> Suppose we have a repo like the one below:
> test.git/
> |-- tracked_file
> |-- untracked_file
> |-- untracked_foo/
> | |-- bar/
> | | |-- bar.txt
> | |-- emptydir/
> | |-- frotz.git/
> | | |-- frotx.txt
> | |-- quux/
> | |-- failedquux.txt
> | |-- quux.txt
> |-- untracked_unreadable_dir/
> | |-- afile
> |-- untracked_some.git/
> |-- some.txt
>
> $ git clean -fd
> Removing untracked_file
> Removing untracked_foo/bar
> Removing untracked_foo/emptydir
> Removing untracked_foo/quux/quux.txt
> warning: failed to remove untracked_foo/quux/failedquux.txt
> warning: failed to remove remove untracked_unreadable_dir/
"remove remove" is a typo, I presume.
> warning: ignoring untracked git repository untracked_foo/frotz.git/
> warning: ignoring untracked git repository untracked_some.git/
If you mean "we report the topmost directory and nothing about
(recursive) contents in it if everything is removed successfully"
(in other words, if we had subdirectories and files inside
untracked_foo/bar/ and we successfully removed all of them, the
above output does not change), it seems quite reasonable.
> Use git clean --force --force to delete all untracked git repositories
But I am not sure if this is ever sane. Especially the one that
removes an embedded repository is suspicious. "git clean" should
not ever touch it with or without --superforce or any other command.
I do not think trying to remove something that cannot be removed due
to filesystem permissions is sensible, either. We simply should treat
such a case a grave error and have the user sort things out, instead
of blindly attempt to "chmod" them ourselves (which may still fail).
Thanks.
^ permalink raw reply
* Re: Feature Request - Hide ignored files before checkout
From: Erik Faye-Lund @ 2012-12-10 9:47 UTC (permalink / raw)
To: Matthew Ciancio; +Cc: Andrew Ardill, git
In-Reply-To: <000001cdd67a$39be9d40$ad3bd7c0$@gmail.com>
On Mon, Dec 10, 2012 at 3:01 AM, Matthew Ciancio
<matthew.ciancio16@gmail.com> wrote:
> Thanks for explaining that Andrew. I guess that was my intention: to have an "ignored file snapshot", but I can see now that it goes against Git's definitions and is not really needed.
>
I have played around with the idea of backing up files deleted by
git-clean in the object database, maintained by a reflog (similar to
git-stash). I did this to protect my code against my fat fingers, but
perhaps this could also have been useful in your case?
https://github.com/kusma/git/tree/work/clean-backup
^ permalink raw reply
* Re: [PATCH v2] cache-tree: invalidate i-t-a paths after generating trees
From: Nguyen Thai Ngoc Duy @ 2012-12-10 11:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <7v7goqcsdy.fsf@alter.siamese.dyndns.org>
On Mon, Dec 10, 2012 at 1:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> diff --git a/cache-tree.c b/cache-tree.c
>> index 28ed657..989a7ff 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -248,6 +248,7 @@ static int update_one(struct cache_tree *it,
>> int missing_ok = flags & WRITE_TREE_MISSING_OK;
>> int dryrun = flags & WRITE_TREE_DRY_RUN;
>> int i;
>> + int to_invalidate = 0;
>>
>> if (0 <= it->entry_count && has_sha1_file(it->sha1))
>> return it->entry_count;
>> @@ -324,7 +325,13 @@ static int update_one(struct cache_tree *it,
>> if (!sub)
>> die("cache-tree.c: '%.*s' in '%s' not found",
>> entlen, path + baselen, path);
>> - i += sub->cache_tree->entry_count - 1;
>> + i--; /* this entry is already counted in "sub" */
>> + if (sub->cache_tree->entry_count < 0) {
>> + i -= sub->cache_tree->entry_count;
>> + to_invalidate = 1;
>> + }
>> + else
>> + i += sub->cache_tree->entry_count;
>
> Hrm. update_one() is prepared to see a cache-tree whose entry count
> is zero (see the context lines in the previous hunk) and the
> invariant for the rest of the code is "if 0 <= entry_count, the
> cached tree is valid; invalid cache-tree has -1 in entry_count.
> More importantly, entry_count negated does not in general express
> how many entries there are in the subtree and does not tell us how
> many index entries to skip.
Yeah I use entry_count for two different things here. In the previous
(unsent) version of the patch I had "entry_count = -1" right after "i
-= entry_count"
>> + if (sub->cache_tree->entry_count < 0) {
>> + i -= sub->cache_tree->entry_count;
>> + sub->cache_tree->entry_count = -1;
>> + to_invalidate = 1;
>> + }
which makes it clearer that the use of negative entry count is only
valid within update_one. Then I changed my mind because it says
'negative means "invalid"' in cache-tree.h. So, put "entry_count = -1"
back or just add another field to struct cache_tree for this?
--
Duy
^ permalink raw reply
* Re: [PATCH] git(1): remove a defunct link to "list of authors"
From: Nguyen Thai Ngoc Duy @ 2012-12-10 12:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vboe2ct9p.fsf@alter.siamese.dyndns.org>
On Mon, Dec 10, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Sat, Dec 8, 2012 at 12:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> * If somebody has a working replacement URL, we could use that
>>>> instead, of course. Takers?
>>>
>>> A possible alternative could be https://www.ohloh.net/p/git/contributors/summary
>>
>> Nice charts!
>
> Yup.
>
> Their numbers seem to be just 'any commit by the author, with
> mailmap applied', and I am of two minds with it. Counting without
> "shortlog --no-merges", depending on the management style of the
> project, tends to credit the integrator too much. Even though
> vetting the patches and choosing when to merge the topics is a
> significant contribution, it isn't *that* big compared to the work
> done by the contributor who took initiative to scratch that itch.
>
> With or without "--no-merges", the big picture you can get out of
> "git shortlog -s -n --since=1.year" does not change very much, but
> the headline numbers give a wrong impression.
These numbers are approximate anyway. Commit counts or the number of
changed lines do not accurately reflect the effort in many cases. And
about merges, in this particular case of Git where the maintainer imo
has done an excellent job as a guard, I'd say it's the credit for
reviewing, not simply merging.
But not using the link is fine too. We can wait for Jeff's patch to be merged.
> And of course, application of the mailmap is very important, if you
> want to get meaningful numbers out of shortlog over a longer period.
--
Duy
^ permalink raw reply
* [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty
From: Erik Faye-Lund @ 2012-12-10 14:42 UTC (permalink / raw)
To: git; +Cc: msysgit, johannes.schindelin, gitster
in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
a check was added to prevent us from retrying to delete a directory
that is both in use and non-empty.
However, this logic was slightly flawed; since we didn't return
immediately, we end up falling out of the retry-loop, but right into
the prompting-loop.
Fix this by setting errno, and guarding the prompting-loop with an
errno-check.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
Here's the second version of this patch, sorry for the slight delay.
compat/mingw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 1eb974f..440224c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -260,6 +260,8 @@ int mingw_rmdir(const char *pathname)
while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
if (!is_file_in_use_error(GetLastError()))
+ errno = err_win_to_posix(GetLastError());
+ if (errno != EACCES)
break;
if (!is_dir_empty(wpathname)) {
errno = ENOTEMPTY;
@@ -275,7 +277,7 @@ int mingw_rmdir(const char *pathname)
Sleep(delay[tries]);
tries++;
}
- while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+ while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
ask_yes_no_if_possible("Deletion of directory '%s' failed. "
"Should I try again?", pathname))
ret = _wrmdir(wpathname);
--
1.8.0.msysgit.0.3.gafa53b0.dirty
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply related
* Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty
From: Junio C Hamano @ 2012-12-10 16:25 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, msysgit, johannes.schindelin
In-Reply-To: <1355150547-8212-1-git-send-email-kusmabite@gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
> a check was added to prevent us from retrying to delete a directory
> that is both in use and non-empty.
>
> However, this logic was slightly flawed; since we didn't return
> immediately, we end up falling out of the retry-loop, but right into
> the prompting-loop.
>
> Fix this by setting errno, and guarding the prompting-loop with an
> errno-check.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>
> Here's the second version of this patch, sorry for the slight delay.
Is this meant for me, or is it to be applied to msysgit and later
somehow fed to me in different form?
I can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you
want me to do, but otherwise this patch does not apply.
>
> compat/mingw.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 1eb974f..440224c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -260,6 +260,8 @@ int mingw_rmdir(const char *pathname)
>
> while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
> if (!is_file_in_use_error(GetLastError()))
> + errno = err_win_to_posix(GetLastError());
> + if (errno != EACCES)
> break;
> if (!is_dir_empty(wpathname)) {
> errno = ENOTEMPTY;
> @@ -275,7 +277,7 @@ int mingw_rmdir(const char *pathname)
> Sleep(delay[tries]);
> tries++;
> }
> - while (ret == -1 && is_file_in_use_error(GetLastError()) &&
> + while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
> ask_yes_no_if_possible("Deletion of directory '%s' failed. "
> "Should I try again?", pathname))
> ret = _wrmdir(wpathname);
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH] Makefile: whitespace style fixes in macro definitions
From: Junio C Hamano @ 2012-12-10 16:37 UTC (permalink / raw)
To: Stefano Lattarini; +Cc: git
In-Reply-To: <0544836357d56c05188941ef5a471605fa6d4881.1355049367.git.stefano.lattarini@gmail.com>
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> Consistently use a single space before and after the "=" (or ":=", "+=",
> etc.) in assignments to make macros. Granted, this was not a big deal,
> but I did find the needless inconsistency quite distracting.
>
> Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
> ---
Makes sense to do this kind of clean-up when these files are
quiescent (and they are).
Thanks.
> Makefile | 56 ++++++++++++++++++++++++++++----------------------------
> config.mak.in | 2 +-
> t/Makefile | 2 +-
> 3 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4ad6fbd..736ecd4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -374,7 +374,7 @@ htmldir = share/doc/git-doc
> ETC_GITCONFIG = $(sysconfdir)/gitconfig
> ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
> lib = lib
> -# DESTDIR=
> +# DESTDIR =
> pathsep = :
>
> export prefix bindir sharedir sysconfdir gitwebdir localedir
> @@ -575,9 +575,9 @@ endif
> export PERL_PATH
> export PYTHON_PATH
>
> -LIB_FILE=libgit.a
> -XDIFF_LIB=xdiff/lib.a
> -VCSSVN_LIB=vcs-svn/lib.a
> +LIB_FILE = libgit.a
> +XDIFF_LIB = xdiff/lib.a
> +VCSSVN_LIB = vcs-svn/lib.a
>
> LIB_H += xdiff/xinclude.h
> LIB_H += xdiff/xmacros.h
> @@ -1139,7 +1139,7 @@ ifeq ($(uname_S),NetBSD)
> endif
> ifeq ($(uname_S),AIX)
> DEFAULT_PAGER = more
> - NO_STRCASESTR=YesPlease
> + NO_STRCASESTR = YesPlease
> NO_MEMMEM = YesPlease
> NO_MKDTEMP = YesPlease
> NO_MKSTEMPS = YesPlease
> @@ -1147,7 +1147,7 @@ ifeq ($(uname_S),AIX)
> NO_NSEC = YesPlease
> FREAD_READS_DIRECTORIES = UnfortunatelyYes
> INTERNAL_QSORT = UnfortunatelyYes
> - NEEDS_LIBICONV=YesPlease
> + NEEDS_LIBICONV = YesPlease
> BASIC_CFLAGS += -D_LARGE_FILES
> ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
> NO_PTHREADS = YesPlease
> @@ -1155,13 +1155,13 @@ ifeq ($(uname_S),AIX)
> PTHREAD_LIBS = -lpthread
> endif
> ifeq ($(shell expr "$(uname_V).$(uname_R)" : '5\.1'),3)
> - INLINE=''
> + INLINE = ''
> endif
> GIT_TEST_CMP = cmp
> endif
> ifeq ($(uname_S),GNU)
> # GNU/Hurd
> - NO_STRLCPY=YesPlease
> + NO_STRLCPY = YesPlease
> NO_MKSTEMPS = YesPlease
> HAVE_PATHS_H = YesPlease
> LIBC_CONTAINS_LIBINTL = YesPlease
> @@ -1187,9 +1187,9 @@ ifeq ($(uname_S),IRIX)
> NEEDS_LIBGEN = YesPlease
> endif
> ifeq ($(uname_S),IRIX64)
> - NO_SETENV=YesPlease
> + NO_SETENV = YesPlease
> NO_UNSETENV = YesPlease
> - NO_STRCASESTR=YesPlease
> + NO_STRCASESTR = YesPlease
> NO_MEMMEM = YesPlease
> NO_MKSTEMPS = YesPlease
> NO_MKDTEMP = YesPlease
> @@ -1203,14 +1203,14 @@ ifeq ($(uname_S),IRIX64)
> NO_REGEX = YesPlease
> NO_FNMATCH_CASEFOLD = YesPlease
> SNPRINTF_RETURNS_BOGUS = YesPlease
> - SHELL_PATH=/usr/gnu/bin/bash
> + SHELL_PATH = /usr/gnu/bin/bash
> NEEDS_LIBGEN = YesPlease
> endif
> ifeq ($(uname_S),HP-UX)
> INLINE = __inline
> - NO_IPV6=YesPlease
> - NO_SETENV=YesPlease
> - NO_STRCASESTR=YesPlease
> + NO_IPV6 = YesPlease
> + NO_SETENV = YesPlease
> + NO_STRCASESTR = YesPlease
> NO_MEMMEM = YesPlease
> NO_MKSTEMPS = YesPlease
> NO_STRLCPY = YesPlease
> @@ -1386,10 +1386,10 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> MKDIR_WO_TRAILING_SLASH = YesPlease
> # RFE 10-120912-4693 submitted to HP NonStop development.
> NO_SETITIMER = UnfortunatelyYes
> - SANE_TOOL_PATH=/usr/coreutils/bin:/usr/local/bin
> - SHELL_PATH=/usr/local/bin/bash
> + SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
> + SHELL_PATH = /usr/local/bin/bash
> # as of H06.25/J06.14, we might better use this
> - #SHELL_PATH=/usr/coreutils/bin/bash
> + #SHELL_PATH = /usr/coreutils/bin/bash
> endif
> ifneq (,$(findstring MINGW,$(uname_S)))
> pathsep = ;
> @@ -1437,7 +1437,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> X = .exe
> SPARSE_FLAGS = -Wno-one-bit-signed-bitfield
> ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
> - htmldir=doc/git/html/
> + htmldir = doc/git/html/
> prefix =
> INSTALL = /bin/install
> EXTLIBS += /mingw/lib/libz.a
> @@ -1559,7 +1559,7 @@ else
> CURL_LIBCURL = -lcurl
> endif
> ifdef NEEDS_SSL_WITH_CURL
> - CURL_LIBCURL += -lssl
> + CURL_LIBCURL += -lssl
> ifdef NEEDS_CRYPTO_WITH_SSL
> CURL_LIBCURL += -lcrypto
> endif
> @@ -1768,7 +1768,7 @@ ifdef OBJECT_CREATION_USES_RENAMES
> endif
> ifdef NO_STRUCT_ITIMERVAL
> COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
> - NO_SETITIMER=YesPlease
> + NO_SETITIMER = YesPlease
> endif
> ifdef NO_SETITIMER
> COMPAT_CFLAGS += -DNO_SETITIMER
> @@ -1920,15 +1920,15 @@ ifneq (,$(XDL_FAST_HASH))
> endif
>
> ifeq ($(TCLTK_PATH),)
> -NO_TCLTK=NoThanks
> +NO_TCLTK = NoThanks
> endif
>
> ifeq ($(PERL_PATH),)
> -NO_PERL=NoThanks
> +NO_PERL = NoThanks
> endif
>
> ifeq ($(PYTHON_PATH),)
> -NO_PYTHON=NoThanks
> +NO_PYTHON = NoThanks
> endif
>
> QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
> @@ -1975,13 +1975,13 @@ PROFILE_DIR := $(CURDIR)
> ifeq ("$(PROFILE)","GEN")
> CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
> EXTLIBS += -lgcov
> - export CCACHE_DISABLE=t
> - V=1
> + export CCACHE_DISABLE = t
> + V = 1
> else
> ifneq ("$(PROFILE)","")
> CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
> - export CCACHE_DISABLE=t
> - V=1
> + export CCACHE_DISABLE = t
> + V = 1
> endif
> endif
>
> @@ -2830,7 +2830,7 @@ git.spec: git.spec.in GIT-VERSION-FILE
> sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
> mv $@+ $@
>
> -GIT_TARNAME=git-$(GIT_VERSION)
> +GIT_TARNAME = git-$(GIT_VERSION)
> dist: git.spec git-archive$(X) configure
> ./git-archive --format=tar \
> --prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar
> diff --git a/config.mak.in b/config.mak.in
> index 69d4838..e8a9bb4 100644
> --- a/config.mak.in
> +++ b/config.mak.in
> @@ -18,7 +18,7 @@ datarootdir = @datarootdir@
> template_dir = @datadir@/git-core/templates
> sysconfdir = @sysconfdir@
>
> -mandir=@mandir@
> +mandir = @mandir@
>
> srcdir = @srcdir@
> VPATH = @srcdir@
> diff --git a/t/Makefile b/t/Makefile
> index 88e289f..3025418 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -6,7 +6,7 @@
> -include ../config.mak.autogen
> -include ../config.mak
>
> -#GIT_TEST_OPTS=--verbose --debug
> +#GIT_TEST_OPTS = --verbose --debug
> SHELL_PATH ?= $(SHELL)
> PERL_PATH ?= /usr/bin/perl
> TAR ?= $(TAR)
^ permalink raw reply
* Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty
From: Erik Faye-Lund @ 2012-12-10 16:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, msysgit, johannes.schindelin
In-Reply-To: <7vr4mxc1rd.fsf@alter.siamese.dyndns.org>
On Mon, Dec 10, 2012 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
>> a check was added to prevent us from retrying to delete a directory
>> that is both in use and non-empty.
>>
>> However, this logic was slightly flawed; since we didn't return
>> immediately, we end up falling out of the retry-loop, but right into
>> the prompting-loop.
>>
>> Fix this by setting errno, and guarding the prompting-loop with an
>> errno-check.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>>
>> Here's the second version of this patch, sorry for the slight delay.
>
> Is this meant for me, or is it to be applied to msysgit and later
> somehow fed to me in different form?
>
> I can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you
> want me to do, but otherwise this patch does not apply.
>
Ugh, you are right. I intended for you to apply it, but I didn't
realize that my patch was based on the msysGit-master, where Karsten's
UTF-8 patches has been applied.
I'm not entirely sure what the best approach would be. The issue is
present in both branches, but we only build installers from the
msysGit-branch. But I think there are other people who builds Git from
the upstream source code, so it would be slightly less annoying for
them if the patch was fixed up and applied. But on the other hand,
that causes some annoyance when (if?) Karsten's UTF-8 patches gets
upstreamed.
Thoughts?
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Soren Brinkmann @ 2012-12-10 17:04 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: Soren Brinkmann, Junio C Hamano, git
In-Reply-To: <CAKJhZwROXsTa4wu-C9rhfGysetL+cZRDECyFUn5VTb833pWzMQ@mail.gmail.com>
Hi Zoltan,
On Sun, Dec 09, 2012 at 10:18:19PM +1100, Zoltan Klinger wrote:
> >> Hrm, following your discussion (ellided above), I would have
> >> expected that you would show
> >>
> >> Removing directory foo/bar
> >> Removing untracked_file1
> >
> > Also it would be nice to have warnings about undeleted directories since this git
> > clean behavior (or the work around to pass -f twice) is not documented.
> > Without a warning you would probably miss that something was _not_ deleted.
>
> Thanks for the feedback. I think you're right. Showing 'foo/bar/bar.txt' in
> the list when 'foo/bar/' directory has been successfully deleted is just noise.
>
> Would like to get some more feedback on the proposed output in case of
> (1) an untracked subdirectory with multiple files where at least one of them
> cannot be removed.
> (2) reporting ignored untracked git subdirectories
>
> Suppose we have a repo like the one below:
> test.git/
> |-- tracked_file
> |-- untracked_file
> |-- untracked_foo/
> | |-- bar/
> | | |-- bar.txt
> | |-- emptydir/
> | |-- frotz.git/
> | | |-- frotx.txt
> | |-- quux/
> | |-- failedquux.txt
> | |-- quux.txt
> |-- untracked_unreadable_dir/
> | |-- afile
> |-- untracked_some.git/
> |-- some.txt
>
> $ git clean -fd
> Removing untracked_file
> Removing untracked_foo/bar
> Removing untracked_foo/emptydir
> Removing untracked_foo/quux/quux.txt
> warning: failed to remove untracked_foo/quux/failedquux.txt
> warning: failed to remove remove untracked_unreadable_dir/
> warning: ignoring untracked git repository untracked_foo/frotz.git/
> warning: ignoring untracked git repository untracked_some.git/
> Use git clean --force --force to delete all untracked git repositories
>
> $ # use forced remove
> $ git clean --force --force -d
> Removing untracked_foo/frotz.git
> Removing untracked_foo/quux/quux.txt
> Removing untracked_some.git/
> warning: failed to remove untracked_foo/quux/failedquux.txt
> warning: failed to remove untracked_unreadable_dir/
>
> Can you see any issues with the proposed output, wording above? If
> everyone is happy,
> I'm going to prepare patch V2 for it.
Looks good to me.
Thanks,
Soren
^ permalink raw reply
* Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty
From: Johannes Schindelin @ 2012-12-10 17:05 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: Junio C Hamano, git, msysgit
In-Reply-To: <CABPQNSZL-edn4izfMuss1-3KbLBSrGm8J08wn0TbETtsn2nn+A@mail.gmail.com>
Hi kusma,
On Mon, 10 Dec 2012, Erik Faye-Lund wrote:
> On Mon, Dec 10, 2012 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Erik Faye-Lund <kusmabite@gmail.com> writes:
> >
> >> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"), a
> >> check was added to prevent us from retrying to delete a directory
> >> that is both in use and non-empty.
> >>
> >> However, this logic was slightly flawed; since we didn't return
> >> immediately, we end up falling out of the retry-loop, but right into
> >> the prompting-loop.
> >>
> >> Fix this by setting errno, and guarding the prompting-loop with an
> >> errno-check.
> >>
> >> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> >> ---
> >>
> >> Here's the second version of this patch, sorry for the slight delay.
> >
> > Is this meant for me, or is it to be applied to msysgit and later
> > somehow fed to me in different form?
> >
> > I can s/_wrmdir/rmdir/;s/wpathname/pathname/ if that is what you
> > want me to do, but otherwise this patch does not apply.
> >
>
> Ugh, you are right. I intended for you to apply it, but I didn't realize
> that my patch was based on the msysGit-master, where Karsten's UTF-8
> patches has been applied.
>
> I'm not entirely sure what the best approach would be. The issue is
> present in both branches, but we only build installers from the
> msysGit-branch. But I think there are other people who builds Git from
> the upstream source code, so it would be slightly less annoying for them
> if the patch was fixed up and applied. But on the other hand, that
> causes some annoyance when (if?) Karsten's UTF-8 patches gets
> upstreamed.
>
> Thoughts?
My preference would be to fix it in both branches. I will fix the merge
conflicts when rebasing onto Junio's master branch next time.
Ciao,
Dscho
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH v2] cache-tree: invalidate i-t-a paths after generating trees
From: Junio C Hamano @ 2012-12-10 17:22 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <CACsJy8CW1UGgQcgHa11XP71XZGaTxytrGmJmrtdNvy=N7cUyMw@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Yeah I use entry_count for two different things here. In the previous
> (unsent) version of the patch I had "entry_count = -1" right after "i
> -= entry_count"
>
>>> + if (sub->cache_tree->entry_count < 0) {
>>> + i -= sub->cache_tree->entry_count;
>>> + sub->cache_tree->entry_count = -1;
>>> + to_invalidate = 1;
>>> + }
>
>
> which makes it clearer that the use of negative entry count is only
> valid within update_one. Then I changed my mind because it says
> 'negative means "invalid"' in cache-tree.h.
But you make it to mean not just 'negative means invalid' but
'negate it and you can learn how many index entries to skip to reach
the entry outside the subtree'. That is what I was wondering about.
I do not think that new invariant does not hold in general; it may
hold true while inside this function, but immediately after somebody
else calls cache_tree_invalidate_path() it won't be true anymore.
Leaking the information to outside callers that can easily be
mistaken as if it may mean something without any comment looks like
we are asking for trouble.
> So, put "entry_count = -1"
> back or just add another field to struct cache_tree for this?
As long as it is made clear with in-code comment that says "negative
entry count, when negated, will give us how many index entries are
covered by this invalid cache tree, only inside this function", and
such a negative entry_count is reset to -1 to avoid leaking out the
value that will soon go stale to the outside world in the "write
this tree out" loop, I think the v2 patch is OK, if not ideal.
If we were to commit to keep the new invariant true throughout the
system, on the other hand, I suspect that a boolean flag "valid" may
help people who keep i-t-a entries around across write-tree calls.
The first if () statement in the update_one() function can check the
validity, and you can say the cache tree is valid even if the
section in the index that is covered by the cache-tree contains
i-t-a entries _after_ you wrote it out as a tree, no? That may make
the semantics a lot cleaner, I suspect.
The new semantics might be:
* update_one() returns negative only when the section of the index
given to it cannot be written out as a tree (i.e. not merged, or
corrupt index);
* update_one() returns the number of entries in the index covered
by the tree, including i-t-a entries (but not REMOVED, as these
entries will not be in the index after the cache-tree has been
written out);
* cache_tree->valid tells if the cache_tree->sha1 is valid; the
section of the index the tree covers may or may not have i-t-a
entries in it;
* cache_tree->entry_count holds the number of index entries the
tree covers, couting i-t-a entries. This field is only meaningful
when cache_tree->valid is true.
or something like that.
I noticed that the recent "ignore i-t-a only while writing trees
instead of erroring out" broke 331fcb5 (git add --intent-to-add: do
not let an empty blob be committed by accident, 2008-11-28) in
another way, by the way. In verify_cache(), there is an unreachable
else clause to warn about "not added yet" entries that should have
been removed but is left behind.
^ permalink raw reply
* Re: [PATCHv2] mingw_rmdir: do not prompt for retry when non-empty
From: Junio C Hamano @ 2012-12-10 17:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Erik Faye-Lund, git, msysgit
In-Reply-To: <alpine.DEB.1.00.1212101804290.32206@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> My preference would be to fix it in both branches. I will fix the merge
> conflicts when rebasing onto Junio's master branch next time.
OK, then I'll queue the following to my tree.
Thanks for a quick turnaround.
-- >8 --
From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Mon, 10 Dec 2012 15:42:27 +0100
Subject: [PATCH] mingw_rmdir: do not prompt for retry when non-empty
in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
a check was added to prevent us from retrying to delete a directory
that is both in use and non-empty.
However, this logic was slightly flawed; since we didn't return
immediately, we end up falling out of the retry-loop, but right into
the prompting-loop.
Fix this by setting errno, and guarding the prompting-loop with an
errno-check.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
compat/mingw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4e63838..28527ab 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -256,6 +256,8 @@ int mingw_rmdir(const char *pathname)
while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
if (!is_file_in_use_error(GetLastError()))
+ errno = err_win_to_posix(GetLastError());
+ if (errno != EACCES)
break;
if (!is_dir_empty(pathname)) {
errno = ENOTEMPTY;
@@ -271,7 +273,7 @@ int mingw_rmdir(const char *pathname)
Sleep(delay[tries]);
tries++;
}
- while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+ while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
ask_yes_no_if_possible("Deletion of directory '%s' failed. "
"Should I try again?", pathname))
ret = rmdir(pathname);
--
1.8.1.rc1.123.gf61cb86
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply related
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Soren Brinkmann @ 2012-12-10 17:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Zoltan Klinger, Soren Brinkmann, git
In-Reply-To: <7v38zecrqc.fsf@alter.siamese.dyndns.org>
Hi,
On Sun, Dec 09, 2012 at 11:04:59PM -0800, Junio C Hamano wrote:
> Zoltan Klinger <zoltan.klinger@gmail.com> writes:
>
> > Would like to get some more feedback on the proposed output in case of
> > (1) an untracked subdirectory with multiple files where at least one of them
> > cannot be removed.
> > (2) reporting ignored untracked git subdirectories
> >
> > Suppose we have a repo like the one below:
> > test.git/
> > |-- tracked_file
> > |-- untracked_file
> > |-- untracked_foo/
> > | |-- bar/
> > | | |-- bar.txt
> > | |-- emptydir/
> > | |-- frotz.git/
> > | | |-- frotx.txt
> > | |-- quux/
> > | |-- failedquux.txt
> > | |-- quux.txt
> > |-- untracked_unreadable_dir/
> > | |-- afile
> > |-- untracked_some.git/
> > |-- some.txt
> >
> > $ git clean -fd
> > Removing untracked_file
> > Removing untracked_foo/bar
> > Removing untracked_foo/emptydir
> > Removing untracked_foo/quux/quux.txt
> > warning: failed to remove untracked_foo/quux/failedquux.txt
> > warning: failed to remove remove untracked_unreadable_dir/
>
> "remove remove" is a typo, I presume.
>
> > warning: ignoring untracked git repository untracked_foo/frotz.git/
> > warning: ignoring untracked git repository untracked_some.git/
>
> If you mean "we report the topmost directory and nothing about
> (recursive) contents in it if everything is removed successfully"
> (in other words, if we had subdirectories and files inside
> untracked_foo/bar/ and we successfully removed all of them, the
> above output does not change), it seems quite reasonable.
>
> > Use git clean --force --force to delete all untracked git repositories
>
> But I am not sure if this is ever sane. Especially the one that
> removes an embedded repository is suspicious. "git clean" should
> not ever touch it with or without --superforce or any other command.
As I mentioned in my email where I reported this incorrect git clean output, I
have a use case where I want git clean to remove embedded repositories.
Whether it is a sane one is probably a different discussion.
Soren
^ permalink raw reply
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-10 18:03 UTC (permalink / raw)
To: Soren Brinkmann; +Cc: Zoltan Klinger, git
In-Reply-To: <5b69a9f1-0860-41da-914c-d55a17e54092@TX2EHSMHS026.ehs.local>
Soren Brinkmann <soren.brinkmann@xilinx.com> writes:
>> > Use git clean --force --force to delete all untracked git repositories
>>
>> But I am not sure if this is ever sane. Especially the one that
>> removes an embedded repository is suspicious. "git clean" should
>> not ever touch it with or without --superforce or any other command.
> As I mentioned in my email where I reported this incorrect git clean output, I
> have a use case where I want git clean to remove embedded repositories.
> Whether it is a sane one is probably a different discussion.
Why is it a different discussion? If something is not sane, the
tool shouldn't encourage users to do such an insane thing.
^ permalink raw reply
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Soren Brinkmann @ 2012-12-10 18:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Soren Brinkmann, Zoltan Klinger, git
In-Reply-To: <7va9tlbx8v.fsf@alter.siamese.dyndns.org>
On Mon, Dec 10, 2012 at 10:03:28AM -0800, Junio C Hamano wrote:
> Soren Brinkmann <soren.brinkmann@xilinx.com> writes:
>
> >> > Use git clean --force --force to delete all untracked git repositories
> >>
> >> But I am not sure if this is ever sane. Especially the one that
> >> removes an embedded repository is suspicious. "git clean" should
> >> not ever touch it with or without --superforce or any other command.
> > As I mentioned in my email where I reported this incorrect git clean output, I
> > have a use case where I want git clean to remove embedded repositories.
> > Whether it is a sane one is probably a different discussion.
>
> Why is it a different discussion? If something is not sane, the
> tool shouldn't encourage users to do such an insane thing.
>
Well, ok. So I have a repository which essentially consists of a bunch of
scripts which then pull sources via git to build root filesystems, busybox,
kernel etc.
So I have the master repository I'm actually interested in. And then all the
other projects which are pulled in to build stuff from.
looking somehow like this:
top.git
|-src
| |-proj1.git
| |-proj2.git
| |-projn.git
|-build
|-proj1
|-proj2
...
Since the scripts are not perfect I usually used 'git clean -xdf' to wipe
everything and build from scratch. And I had to experience that the git clean
behavior somehow changed recently and the 'projn.git' directories were no longer
removed anymore, despite git indicating otherwise in its output.
So, I think having 'git clean -ff' removing embedded git repos is okay. But either
way, the output of git clean should match what it is doing. And at least tell me
if it didn't remove certain dirs or files.
Soren
^ permalink raw reply
* Use of a mailmap file with git-log
From: Rich Midwinter @ 2012-12-10 18:22 UTC (permalink / raw)
To: git
Hi
I'm working on a project for a large organisation that wants to make
widespread use of git and the mailmap feature.
This seems to be supported by default in git-shortlog but not git-log
(and other variants) without specifying custom formats, which isn't
really something I want to try and 'fix' across the organisation. Is
there a reason for this feature omission or has it just evolved that
way and could it be fixed?
Thanks
Rich
^ permalink raw reply
* Re: Use of a mailmap file with git-log
From: Junio C Hamano @ 2012-12-10 18:48 UTC (permalink / raw)
To: Rich Midwinter; +Cc: git
In-Reply-To: <CALKB1SXdNVsQop5VYmShOMx93+j5SPdkGF9yNU5k7nXg87TwMw@mail.gmail.com>
Rich Midwinter <rich.midwinter@gmail.com> writes:
> I'm working on a project for a large organisation that wants to make
> widespread use of git and the mailmap feature.
>
> This seems to be supported by default in git-shortlog but not git-log
> (and other variants) without specifying custom formats, which isn't
> really something I want to try and 'fix' across the organisation. Is
> there a reason for this feature omission or has it just evolved that
> way and could it be fixed?
I think it was pretty much the latter, but people may already be
depending on the command to give them the "true as recorded back
then" names in the output. A fix may have to involve inventing a
new option "log --use-mailmap" that is explicitly given from the
command line.
^ permalink raw reply
* Re: [PATCH] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2012-12-10 18:53 UTC (permalink / raw)
To: Soren Brinkmann; +Cc: Zoltan Klinger, git
In-Reply-To: <aecaf65e-2b7f-4309-a7b5-622c7779de17@DB3EHSMHS018.ehs.local>
Soren Brinkmann <soren.brinkmann@xilinx.com> writes:
> But either
> way, the output of git clean should match what it is doing. And at least tell me
> if it didn't remove certain dirs or files.
Oh, no question about that part. I was reacting to --force --force
in general, and an unrelated git repository inside a working tree is
just a subset of the issue.
^ permalink raw reply
* Re: Use of a mailmap file with git-log
From: Junio C Hamano @ 2012-12-10 19:43 UTC (permalink / raw)
To: git; +Cc: Rich Midwinter
In-Reply-To: <7v38zdbv6d.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Rich Midwinter <rich.midwinter@gmail.com> writes:
>
>> I'm working on a project for a large organisation that wants to make
>> widespread use of git and the mailmap feature.
>>
>> This seems to be supported by default in git-shortlog but not git-log
>> (and other variants) without specifying custom formats, which isn't
>> really something I want to try and 'fix' across the organisation. Is
>> there a reason for this feature omission or has it just evolved that
>> way and could it be fixed?
>
> I think it was pretty much the latter, but people may already be
> depending on the command to give them the "true as recorded back
> then" names in the output. A fix may have to involve inventing a
> new option "log --use-mailmap" that is explicitly given from the
> command line.
If somebody wants to do this, I think the overall design should go
like this:
* We may want to rewrite blame.c::get_ac_line() and the code in
pretty.c::pp_user_info() that parse author/committer lines by
using ident.c::split_ident_line() for better code reuse as a
preparation step before all of the below.
* We may want to lift the buffer length limit from the implementation
of mailmap.c::map_user() by using the strbuf API as a preparation
step before all of the below.
* We may also want to rethink its signature (we may want to get a
single strbuf and have the function parse out "Name <mail>"; I
didn't check the existing callers to see if that would make it
easier to use, and if it does not, this obviously shouldn't be done)
as a preparation step before all of the below.
* Introduce a new "struct string_list *mailmap" member to "struct
pretty_print_context" and "struct rev_info" (default to NULL);
* In log-tree.c::show_log(), copy opt->mailmap to ctx.mailmap;
* Update pretty.c::pp_user_info() to convert the email address on
"line" (between the beginning and "namelen") by calling
map_user() immediately after it parses time/tz out, and adjust
the remainder of the function to use it, when pp->mailmap is
present;
* Teach log.c::cmd_log_init_finosh() about "--use-mailmap" option.
Allocate one "struct string_list" instance and use read_mailmap()
on it when the option is used, and store it in rev->mailmap.
^ 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