* Re: git mergetool indefinite hang on version 2.10+
From: Jeff King @ 2016-11-09 23:13 UTC (permalink / raw)
To: Mike Dacre; +Cc: git
In-Reply-To: <CAPd9ww_B7gCxvSuuBzH9AbnLsOF1bC_2+mfk0sVfLFF7YHWvNA@mail.gmail.com>
On Wed, Nov 09, 2016 at 02:54:59PM -0800, Mike Dacre wrote:
> I have no idea how to debug this. As of git version 2.10.0 git hangs
> indefinitely when I run `git mergetool`, the result is the same if I
> run `git mergetool --tool-help`.
Mergetool is a shell script. Try setting GIT_TRACE=1 in the environment
to see which git programs it's running. Of course it may be
hanging in a non-git program, too. Try:
PATH=$(git --exec-path):$PATH
sh -x $(which git-mergetool) --tool-help
to get a dump from the shell.
> I am on Arch Linux, with all of my software updated to the latest
> versions as of this morning. git 2.9.3 and lower works perfectly.
>
> Any ideas?
If it works on v2.9.3 and the bug reproduces, you should be able to use
"git bisect" to find the commit that introduces the problem. There
weren't a lot of changes to the shell scripts in that time frame, so
just as a guess, it may be related to d323c6b64 (i18n: git-sh-setup.sh:
mark strings for translation, 2016-06-17), which was the only
significant change in that time.
-Peff
^ permalink raw reply
* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
From: Junio C Hamano @ 2016-11-09 23:13 UTC (permalink / raw)
To: Ian Jackson; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras
In-Reply-To: <22562.65461.845411.29907@chiark.greenend.org.uk>
Ian Jackson <ijackson@chiark.greenend.org.uk> writes:
>> But I think the right
>> place to do so would be Edit -> Preferences menu in Gitk, and the
>> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
>> gitk-specific place.
>
> This is not correct, because as I have explained, this should be a
> per-tree configuration:
I do not have fundamental opposition to make it part of .git/config,
but the name "gitk.something" or if you are enhancing git-gui at the
time perhaps "gui.something" would be appropriate.
But it is still silly to have this kind of information that is very
specific to Gitk in two places, one that is pretty Gitk specific
that core-git does not know anything about, the other that are part
of the configuration storage of the core-git. In the longer term,
it is necessary for them to be accessible from gitk's "Edit ->
Preferences" mechanism somehow, I would think, rather than forcing
users to sometimes go to GUI to tweak and sometimes run "git config".
^ permalink raw reply
* Re: [PATCH 0/3] gitk: memory consumption improvements
From: Junio C Hamano @ 2016-11-09 23:04 UTC (permalink / raw)
To: Markus Hitter; +Cc: git@vger.kernel.org, Paul Mackerras
In-Reply-To: <8eac2a5b-071f-6d17-4d81-0744db16910d@jump-ing.de>
Markus Hitter <mah@jump-ing.de> writes:
> Am 08.11.2016 um 22:37 schrieb Junio C Hamano:
>> Are all semi-modern Tcl/Tk in service have this -undo thing so that
>> we can pass unconditionally to the text widget like the patch does?
>
> Good point. As far as my research goes, this flag was introduced in Nov. 2001:
>
> http://core.tcl.tk/tk/info/5265df93d207cec0
Sounds safe enough then ;-)
Thanks for an additional research.
^ permalink raw reply
* Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Junio C Hamano @ 2016-11-09 22:58 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CACsJy8BoEXDjwe=ZX5ZOC_mvaMjYrB3i7wcMmiOP3mm5-rwC5Q@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Let's err on the safe side and disable symlinks to outside repo by
> default (or even all symlinks on .gitattributes and .gitignore as the
> first step)
>
> What I learned from my changes in .gitignore is, if we have not
> forbidden something, people likely find some creative use for it.
Yup. Supporting any symlink in-tree is like requiring Git to be
used only on symlink-capable filesystems. Not allowing it sounds
like a very sensible option and unlike true contents, there is no
downside to give that limitation to things like .git<anything>.
Shouldn't we do the same for .gitmodules while we are at it?
^ permalink raw reply
* Re: Regarding "git log" on "git series" metadata
From: Junio C Hamano @ 2016-11-09 22:57 UTC (permalink / raw)
To: Josh Triplett
Cc: Jacob Keller, Christian Couder, git, Shawn O. Pierce, Jeff King,
Nguyen Thai Ngoc Duy, Mike Hommey
In-Reply-To: <20161106173311.lqoxxgcklx4jlrg7@x>
Josh Triplett <josh@joshtriplett.org> writes:
>> This would definitely need protocol extension when transferring
>> objects across repositories.
>
> It'd also need a repository format extension locally. Otherwise, if you
> ever touched that repository with an older git (or a tool built on an
> older libgit2 or JGit or other library), you could lose data.
True. Thanks for sanity-checking me.
^ permalink raw reply
* git mergetool indefinite hang on version 2.10+
From: Mike Dacre @ 2016-11-09 22:54 UTC (permalink / raw)
To: git
Hi all,
I have no idea how to debug this. As of git version 2.10.0 git hangs
indefinitely when I run `git mergetool`, the result is the same if I
run `git mergetool --tool-help`.
The result is identical regardless of which version of vim I have
installed, or where vim in installed. It is also the same if I remove
my .gitconfig file.
I am on Arch Linux, with all of my software updated to the latest
versions as of this morning. git 2.9.3 and lower works perfectly.
Any ideas?
Thanks,
Mike
^ permalink raw reply
* Re: [PATCHv2 32/36] pathspec: allow querying for attributes
From: Stefan Beller @ 2016-11-09 22:42 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Brandon Williams, Git Mailing List
In-Reply-To: <CACsJy8BKwvaw8CsorZhoYDw6gxoosv0gPRRpdwW87+YMASCVcA@mail.gmail.com>
On Wed, Nov 9, 2016 at 1:57 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, Oct 29, 2016 at 1:54 AM, Stefan Beller <sbeller@google.com> wrote:
>> The pathspec mechanism is extended via the new
>> ":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
>> requires paths to not just match the given pattern but also have the
>> specified attrs attached for them to be chosen.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> Documentation/glossary-content.txt | 20 +++++
>> dir.c | 35 ++++++++
>
> Pathspec can be processed in a couple more places. The big two are
> match_pathspec and tree_entry_interesting, the former traverses a list
> while the latter does a tree. You don't have to implement attr
> matching in tree_entry_interesting right now because nobody needs it,
> probably. But you need to make sure if somebody accidentally calls
> tree_entry_interesting with an attr pathspec, then it should
> die("BUG"), not silently ignore attr.
>
I am looking into this now.
^ permalink raw reply
* Re: [PATCH] sequencer: shut up clang warning
From: Jakub Narębski @ 2016-11-09 22:28 UTC (permalink / raw)
To: Johannes Schindelin, git
Cc: Junio C Hamano, Torsten Bögershausen, Lars Schneider,
Jeff King
In-Reply-To: <efbba4b32515fed7096c1c81dbe97eedd44083b0.1478699713.git.johannes.schindelin@gmx.de>
W dniu 09.11.2016 o 14:56, Johannes Schindelin pisze:
> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
>
> comparison of constant 2 with expression of type
> 'const enum todo_command' is always true
>
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
>
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.
>
> Noticed by Torsten Bögershausen.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f3..f80e9c0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
>
> enum todo_command {
> TODO_PICK = 0,
> - TODO_REVERT
> + TODO_REVERT,
> + TODO_INVALID
> };
Why not name it TODO_N, or N_TODO, or something like that?
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines
From: Jakub Narębski @ 2016-11-09 22:25 UTC (permalink / raw)
To: Vasco Almeida, Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, David Aguilar
In-Reply-To: <1478710943.28771.4.camel@sapo.pt>
W dniu 09.11.2016 o 18:02, Vasco Almeida pisze:
> A Ter, 08-11-2016 às 17:06 -0800, Junio C Hamano escreveu:
>> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>>> +sub comment_lines {
>>> + my $comment_line_char = config("core.commentchar") || '#';
>>> + return prefix_lines("$comment_line_char ", @_);
>>> +}
>>> +
>>
>> This makes it appear as if comment_lines can take arbitrary number
>> of strings as its arguments (because the outer caller just passes @_
>> thru), but in fact because prefix_lines ignores anything other than
>> $_[0] and $_[1], only the first parameter given to comment_lineS sub
>> is inspected for lines in it and the prefix-char prefixed at the
>> beginning of each of them.
>>
>> Which is not a great interface, as it is quite misleading.
>>
>> Perhaps
>>
>> prefix_lines("#", join("\n", @_));
>>
>> or something like that may make it less confusing.
>
> I prefer to have like this instead
>
> sub prefix_lines {
> my $prefix = shift;
> my $string = join("\n", @_);
> $string =~ s/^/$prefix/mg;
> return $string;
> }
>
> So both subroutines can take several strings as arguments.
I like the interface, but the implementation looks a bit inefficient.
Why not simply:
sub prefix_lines {
my $prefix = shift;
return "$prefix" . join("\n$prefix", @_) . "\n";
}
That is, if we can assume that those lines are not terminated by
newlines themselves.
If they can be (but cannot have embedded newlines), then
sub prefix_lines {
my $prefix = shift;
return "$prefix" . join("\n$prefix", map(chomp, @_)) . "\n";
}
If those strings can contain embedded newlines (so that they can be
called as in Junio example), then your solution is a must-be
sub prefix_lines {
my $prefix = shift;
my $string = join("\n", @_);
$string =~ s/^/$prefix/mg;
return $string;
}
Well, nevermind then
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH 32/36] pathspec: allow querying for attributes
From: Junio C Hamano @ 2016-11-09 22:25 UTC (permalink / raw)
To: Stefan Beller; +Cc: Duy Nguyen, Git Mailing List, Brandon Williams
In-Reply-To: <CAGZ79kZ=9QeZLKKrH27U2iE9x3WxgVe4RvCZpbdzZriMArV6Sg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Wed, Nov 9, 2016 at 1:45 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> ...
>>> The strategy this round takes to make it unnecessary to punt
>>> preloading (i.e. dropping "pathspec: disable preload-index when
>>> attribute pathspec magic is in use" patch the old series had) is to
>>> make the attribute subsystem thread-safe. But that thread-safety in
>>> the initial round is based on a single Big Attribute Lock, so it may
>>> turn out that the end result performs better for this codepath if we
>>> did not to make any call into the attribute subsystem.
>>
>> It does sound good and I want to say "yes please do this", but is it
>> making pathspec api a bit more complex (to express "assume all
>> attr-related criteria match")? I guess we can have an api to simply
>> filter out attr-related magic (basically set attr_match_nr back to
>> zero) then pass a safe (but more relaxing) pathspec to the threaded
>> code. That would not add big maintenance burden.
>
> So with the current implementation, we already have the shortcut as:
>
> if (item->attr_match_nr && !match_attrs(name, namelen, item))
>
> i.e. if attr_match_nr is zero, we do not even look at the mutexes and such,
> so I am not sure what you intend to say in this email?
I am not sure what relevance the "we call into attribute subsystem
only when there is any need to check attributes" obvious short-cut
has to what is being discussed.
The issue is specific to what preloading is about. It is merely an
attempt to run cheap checks that could be easily multi-threaded with
multiple threads early in the program that we _know_ we would need
to eventually refresh the index before doing some interesting work.
A full refresh_index() will be done eventually, and because it needs
to trigger thread-unsafe part of the API, it needs to be done in the
main thread. Doing the preload allows us to mark index entries that
do not have to be scanned again in the upcoming refresh_index()
call. It is OK for preload-index.c::preload_thread() to skip and
not mark some index entries (iow, its sole purpose is to leave a
note in each index entry "this is fresh, you do not need to look at
it again", and it can choose to skip an entry, which essentially
means "this I didn't check, so you, refresh_index(), need to check
yourself").
preload_thread() for example skips index entries that needs to
trigger "racy Git avoidance" logic that is heavyweight (it has to go
to the filesystem and the object store), and it is a sensible thing
to skip because they are rather rare.
The message by Duy you are responding to was his response to me who
wondered if the attribute based pathspec match also falls into the
same category. Just like racy Git code was deemed too heavyweight
to be called from preloading codepath and CE_MATCH_RACY_IS_DIRTY bit
was added as a way to ask ie_match_stat() API to avoid it (and hence
we are skipping, the caller is also telling "if you suspect a racy,
without checking for real, just answer 'I cannot say it is
clean/fresh'"), if we invent a new flag and pass it through
match_pathspec() down to match_pathspec_item() and have that if()
statement you quoted also skip match_attrs() for a pathspec element
with attribute based narrowing (as we are skipping, the flag may
also have say "instead of checking the attributes, just pretend that
the path did not satisfy the attribute narrowing"), would it benefit
the overall performance?
To answer Duy's "would it make sense to force the caller to create a
new pathspec from an existing one by filtering out pathspec elements
with attr-based narrowing?" question, I do not think it does.
^ permalink raw reply
* [PATCH] remote-curl: don't hang when a server dies before any output
From: David Turner @ 2016-11-09 22:18 UTC (permalink / raw)
To: git, spearce; +Cc: David Turner
In the event that a HTTP server closes the connection after giving a
200 but before giving any packets, we don't want to hang forever
waiting for a response that will never come. Instead, we should die
immediately.
One case where this happens is when attempting to fetch a dangling
object by SHA.
Still to do: it would be good to give a better error message
than "fatal: The remote end hung up unexpectedly".
Signed-off-by: David Turner <dturner@twosigma.com>
---
Note: if you run t5551 before applying the code patch, the second new
test will hang forever.
FWIW, I also saw this kind of hang at Twitter from time to time, but I
was never able to reliably reproduce it there, and thus never able to
fix it. I suspect that a bad load balancer might have been killing
connections just after the headers, but this is pure speculation.
I am sorry that this patch does not provide a more useful error
message. For us, it is important to fix the hangs (so that we can
retry on another server, for instance), but less important to be
user-friendly (since we were only seeing these with automated
processes). I hope that someone who actually understands the http
code, and has some time, could help improve this aspect of the code.
If not, then at least we won't have inexplicable hangs.
remote-curl.c | 8 ++++++++
t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/remote-curl.c b/remote-curl.c
index f14c41f..ee44236 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -400,6 +400,7 @@ struct rpc_state {
size_t pos;
int in;
int out;
+ int any_written;
struct strbuf result;
unsigned gzip_request : 1;
unsigned initial_buffer : 1;
@@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
{
size_t size = eltsize * nmemb;
struct rpc_state *rpc = buffer_;
+ if (size)
+ rpc->any_written = 1;
write_or_die(rpc->in, ptr, size);
return size;
}
@@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
+
+ rpc->any_written = 0;
err = run_slot(slot, NULL);
if (err == HTTP_REAUTH && !large_request) {
credential_fill(&http_auth);
@@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)
if (err != HTTP_OK)
err = -1;
+ if (!rpc->any_written)
+ err = -1;
+
curl_slist_free_all(headers);
free(gzip_body);
return err;
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 1ec5b27..43665ab 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '
test_line_count = 2 posts
'
+test_expect_success 'test allowreachablesha1inwant' '
+ test_when_finished "rm -rf test_reachable.git" &&
+ server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+ git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+ git init --bare test_reachable.git &&
+ git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
+ git -C test_reachable.git fetch origin "$master_sha"
+'
+
+test_expect_success 'test allowreachablesha1inwant with unreachable' '
+ test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
+
+ #create unreachable sha
+ echo content >file2 &&
+ git add file2 &&
+ git commit -m two &&
+ git push public HEAD:refs/heads/doomed &&
+ git push public :refs/heads/doomed &&
+
+ server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+ git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+ git init --bare test_reachable.git &&
+ git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
+ test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
+'
+
test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
--
2.8.0.rc4.22.g8ae061a
^ permalink raw reply related
* Re: [git-for-windows] [ANNOUNCE] Prerelease: Git for Windows v2.11.0-rc0
From: Lars Schneider @ 2016-11-09 22:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git-for-windows, Git Mailing List, me
In-Reply-To: <alpine.DEB.2.20.1611051025030.3108@virtualbox>
On 05 Nov 2016, at 10:50, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> Dear Git users,
>
> I finally got around to rebase the Windows-specific patches (which seem to
> not make it upstream as fast as we get new ones) on top of upstream Git
> v2.11.0-rc0, and to bundle installers, portable Git and MinGit [*1*]:
>
> https://github.com/git-for-windows/git/releases/tag/v2.11.0-rc0.windows.1
I tested a new feature in 2.11 on Windows today and it failed. After some
confusion I realized that the feature is not on your 2.11 branch. Consider this:
My last patch on your 2.11 branch is
"pack-protocol: fix maximum pkt-line size" (7841c4):
https://github.com/git-for-windows/git/commits/v2.11.0-rc0.windows.1?author=larsxschneider
My last patch on the Git 2.11 branch is
"read-cache: make sure file handles are not inherited by child processes" (a0a6cb):
https://github.com/git/git/commits/v2.11.0-rc0?author=larsxschneider
Do you have a clue what is going on?
Thanks,
Lars
^ permalink raw reply
* Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Jeff King @ 2016-11-09 20:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <xmqqd1i49yj1.fsf@gitster.mtv.corp.google.com>
On Wed, Nov 09, 2016 at 12:41:22PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > For matching specific names, we have to deal with case-folding. It's
> > easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> > this is actually protection against malicious repositories, we have to
> > match all of the horrible filesystem-specific junk that we did for
> > ".git".
> >
> > Symlinks are likewise tricky.
>
> Wouldn't it be the simplest to say these:
>
> (1) The code attempts to read ".gitignore" (or ".git<something>")
> in general from the filesystem, or the index, or a tree. No
> case permutations are attempted.
>
> (2) When the code tries to do the above, we open with nofollow (or
> protect racily with lstat(2) which may be the best we could do)
> when reading from the filesystem, or check the ce_mode type
> when reading from the index or from a tree, and ignore if the
> path we are using is a symbolic link.
>
> That way, case funny filesystems that cause trouble like the ".git"
> thing would not have a chance to interfere and fool us, no?
That's what my series does. The question is just whether people will be
unhappy with (2), because they are using symbolic links in their
.gitignore files.
-Peff
^ permalink raw reply
* Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Junio C Hamano @ 2016-11-09 20:41 UTC (permalink / raw)
To: Jeff King; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <20161108222127.mejb74maewzhn3qg@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> For matching specific names, we have to deal with case-folding. It's
> easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> this is actually protection against malicious repositories, we have to
> match all of the horrible filesystem-specific junk that we did for
> ".git".
>
> Symlinks are likewise tricky.
Wouldn't it be the simplest to say these:
(1) The code attempts to read ".gitignore" (or ".git<something>")
in general from the filesystem, or the index, or a tree. No
case permutations are attempted.
(2) When the code tries to do the above, we open with nofollow (or
protect racily with lstat(2) which may be the best we could do)
when reading from the filesystem, or check the ce_mode type
when reading from the index or from a tree, and ignore if the
path we are using is a symbolic link.
That way, case funny filesystems that cause trouble like the ".git"
thing would not have a chance to interfere and fool us, no?
^ permalink raw reply
* Re: [PATCH v2] rebase: add --forget to cleanup rebase, leave everything else untouched
From: Junio C Hamano @ 2016-11-09 20:12 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <20161109091131.17933-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> ---
> v2 changes just the subject line
That's not sufficient, is it? What you did in the documentation
would raise the same "Hmph, is this only about HEAD?" and unlike the
commit subject, it will carve it in stone for end-users.
^ permalink raw reply
* Cleaning ignored files
From: Roman Terekhov @ 2016-11-09 18:23 UTC (permalink / raw)
To: git
Hi,
I want to ask about git clean -dXf command behaviour.
I do the following:
$ mkdir gitignore_test
$ cd gitignore_test/
$ git init
Initialized empty Git repository in ~/gitignore_test/.git/
$ echo *.sln > .gitignore
$ git add .gitignore
$ git commit -m "add gitignore"
[master (root-commit) ef78a3c] add gitignore
1 file changed, 1 insertion(+)
create mode 100644 .gitignore
$ mkdir src
$ touch test.sln
$ touch src/test.sln
$ tree
.
├── src
│ └── test.sln
└── test.sln
1 directory, 2 files
$ git clean -dXf
Removing test.sln
$ tree
.
└── src
└── test.sln
1 directory, 1 file
Why git clean -dXf does not remove all my test.sln files, but just one of them?
Roman Terekhov
^ permalink raw reply
* Re: [PATCH 32/36] pathspec: allow querying for attributes
From: Stefan Beller @ 2016-11-09 18:08 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Git Mailing List, Brandon Williams
In-Reply-To: <CACsJy8C2MLg4ncLBXXJGf+=mPF_rRoKs2vN6=+chZeNeXWZsbg@mail.gmail.com>
On Wed, Nov 9, 2016 at 1:45 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> The reason why I am bringing this up in this discussion thread on
>> this patch is because I wonder if we would benefit by a similar
>> "let's not do too involved things and be cheap by erring on the safe
>> and lazy side" strategy in the call to ce_path_match() call made in
>> this function to avoid making calls to the attr subsystem.
>>
>> In other words, would it help the system by either simplifying the
>> processing done or reducing the cycle spent in preload_thread() if
>> we could tell ce_path_match() "A pathspec we are checking may
>> require not just the pattern to match but also attributes given to
>> the path to satisfy the criteria, but for the purpose of preloading,
>> pretend that the attribute satisfies the match criteria" (or
>> "pretend that it does not match"), thereby not having to make any
>> call into the attribute subsystem at all from this codepath?
>>
>> The strategy this round takes to make it unnecessary to punt
>> preloading (i.e. dropping "pathspec: disable preload-index when
>> attribute pathspec magic is in use" patch the old series had) is to
>> make the attribute subsystem thread-safe. But that thread-safety in
>> the initial round is based on a single Big Attribute Lock, so it may
>> turn out that the end result performs better for this codepath if we
>> did not to make any call into the attribute subsystem.
>
> It does sound good and I want to say "yes please do this", but is it
> making pathspec api a bit more complex (to express "assume all
> attr-related criteria match")? I guess we can have an api to simply
> filter out attr-related magic (basically set attr_match_nr back to
> zero) then pass a safe (but more relaxing) pathspec to the threaded
> code. That would not add big maintenance burden.
>
So with the current implementation, we already have the shortcut as:
if (item->attr_match_nr && !match_attrs(name, namelen, item))
i.e. if attr_match_nr is zero, we do not even look at the mutexes and such,
so I am not sure what you intend to say in this email?
^ permalink raw reply
* Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines
From: Vasco Almeida @ 2016-11-09 17:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <xmqqshr19oxp.fsf@gitster.mtv.corp.google.com>
A Ter, 08-11-2016 às 17:06 -0800, Junio C Hamano escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>
> >
> > Add subroutines prefix_lines and comment_lines.
> >
> > Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> > ---
> > perl/Git.pm | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/perl/Git.pm b/perl/Git.pm
> > index b2732822a..17be59fb7 100644
> > --- a/perl/Git.pm
> > +++ b/perl/Git.pm
> > @@ -1438,6 +1438,29 @@ sub END {
> >
> > } # %TEMP_* Lexical Context
> >
> > +=item prefix_lines ( PREFIX, STRING )
> > +
> > +Prefixes lines in C<STRING> with C<PREFIX>.
> > +
> > +=cut
> > +
> > +sub prefix_lines {
> > + my ($prefix, $string) = @_;
> > + $string =~ s/^/$prefix/mg;
> > + return $string;
> > +}
> > +
> > +=item comment_lines ( STRING )
> > +
> > +Comments lines following core.commentchar configuration.
> > +
> > +=cut
> > +
> > +sub comment_lines {
> > + my $comment_line_char = config("core.commentchar") || '#';
> > + return prefix_lines("$comment_line_char ", @_);
> > +}
> > +
>
> This makes it appear as if comment_lines can take arbitrary number
> of strings as its arguments (because the outer caller just passes @_
> thru), but in fact because prefix_lines ignores anything other than
> $_[0] and $_[1], only the first parameter given to comment_lineS sub
> is inspected for lines in it and the prefix-char prefixed at the
> beginning of each of them.
>
> Which is not a great interface, as it is quite misleading.
>
> Perhaps
>
> prefix_lines("#", join("\n", @_));
>
> or something like that may make it less confusing.
I prefer to have like this instead
sub prefix_lines {
my $prefix = shift;
my $string = join("\n", @_);
$string =~ s/^/$prefix/mg;
return $string;
}
So both subroutines can take several strings as arguments.
^ permalink raw reply
* Re: 2.11.0-rc1 will not be tagged for a few days
From: Jeff King @ 2016-11-09 16:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Johannes Schindelin, Lars Schneider
In-Reply-To: <xmqqoa1p9nf0.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 08, 2016 at 10:29:07PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'm collecting v2.11-rc1 topics in the "refs/heads/for-junio/" section
> > of git://github.com/peff/git.git.
> >
> > I've also got proposed merges for "master" there, though note that none
> > of the topics has actually cooked at all in next (the fixes are trivial
> > enough that it may be OK, though).
>
> I am not quite back up at full steam yet, but I expect I'd be more
> or less functioning tomorrow, so I'll fetch them from your tree and
> continue.
It looks like Johannes prefers replacements for some of the fixes from
yesterday. Just FYI that you probably do not want to take the "master" I
pushed there literally, but instead just view it as a proposed list of
topics.
I did not really expect you to take it literally in the first place.
It's just that I found myself writing up notes like "this should be
merged", and it occurred to me that I could communicate the same things
by sending you a proposed history. So I'm curious if you find dissecting
it via "git log" more or less convenient than a list of notes. :)
-Peff
^ permalink raw reply
* Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Jeff King @ 2016-11-09 16:45 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8ASMBk+Yak7LyybANFYkoU_Poi1ZGY=ufKtq1vSkoYCXQ@mail.gmail.com>
On Wed, Nov 09, 2016 at 04:22:12PM +0700, Duy Nguyen wrote:
> > Symlinks are likewise tricky. If we see that a symlink points to
> > "foo/../bar", then we don't know if it leaves the repository unless we
> > also look at "foo" to see if it is also a symlink. So you really end up
> > having to resolve the symlink yourself (and when checking out multiple
> > files, there's an ordering dependency).
>
> We do have this dependency problem right now (e.g. files A and
> .gitattributes are checked out at the same time and .gitattributes has
> some attribute on A). It looks like we resolve it by reading the index
> version at checkout time. We probably can do the same for gitattribute
> symlinks.
Right, but then we can't use filesystem functions like realpath() to do
the lookup. I guess we could do a pass after the checkout is done to
"fix" any out-of-tree symlinks we just created.
This is exactly the sort of complexity I was trying to avoid with my
original series. :)
If that isn't an option, I think I prefer something like the
core.saneSymlinks approach I mentioned. It has the additional bonus of
protecting not just git commands, but other commands that might inspect
the filesystem.
> > So one reasonable fix might be to have a config option like
> > "core.saneSymlinks" that enforces both of those rules for _all_ symlinks
> > that we checkout to the working tree. And it could either refuse to
> > check them out, or replace them with a file containing the symlink
> > content (as we do on systems that don't support symlinks, IIRC).
>
> I wonder if anyone want core.saneSymlinks on, but they have some links
> that do not meet the above checks and still want to follow them
> anyway. One way to add such an exception is mark the path with an
> attribute "follow". Yeah I have a dependency loop :(
That could come later if somebody wants it, I think (especially if the
config option is not on by default). I have a feeling that callers will
either care about out-of-tree symlinks or not. Trusting the repository
to say "but these ones are OK" doesn't work for the paranoid ones, and
everybody else just assumes the repository is sane.
-Peff
^ permalink raw reply
* Re: [PATCH] sequencer: shut up clang warning
From: Jeff King @ 2016-11-09 15:37 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Torsten Bögershausen, Lars Schneider
In-Reply-To: <efbba4b32515fed7096c1c81dbe97eedd44083b0.1478699713.git.johannes.schindelin@gmx.de>
On Wed, Nov 09, 2016 at 02:56:25PM +0100, Johannes Schindelin wrote:
> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
>
> comparison of constant 2 with expression of type
> 'const enum todo_command' is always true
>
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
>
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.
This does silence the warning.
I slightly prefer mine because:
1. It does not carry an implicit requirement that TODO_INVALID remain
the final enum value.
2. It also protects the range check against a negative enum value.
But this is code that is getting changed later by you anyway, and I do
not care that strongly.
-Peff
^ permalink raw reply
* Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early
From: Jeff King @ 2016-11-09 15:31 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <mvmzil8btzb.fsf@hawking.suse.de>
On Wed, Nov 09, 2016 at 03:36:40PM +0100, Andreas Schwab wrote:
> On Nov 09 2016, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > The reason why we do not ignore kill errors is that we want to make sure
> > that the script *actually ran*. Otherwise, the thing we need to test here
> > does not necessarily get tested.
>
> That can be tested by looking for the pid file.
I agree that makes the intent a lot more obvious. Having a necessary
condition of the test stuffed into a test_when_finished block seems
counter-intuitive.
-Peff
^ permalink raw reply
* Re: [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions
From: Christian Couder @ 2016-11-09 14:47 UTC (permalink / raw)
To: Duy Nguyen
Cc: Git Mailing List, Junio C Hamano,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CACsJy8AisF2ZVs7JdnVFp5wdskkbVQQQ=DBq5UzE1MOsCfBMtQ@mail.gmail.com>
On Wed, Nov 9, 2016 at 10:24 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>>> On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>>> On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
>>>> <christian.couder@gmail.com> wrote:
>>>>> +void remove_split_index(struct index_state *istate)
>>>>> +{
>>>>> + if (istate->split_index) {
>>>>> + /*
>>>>> + * can't discard_split_index(&the_index); because that
>>>>> + * will destroy split_index->base->cache[], which may
>>>>> + * be shared with the_index.cache[]. So yeah we're
>>>>> + * leaking a bit here.
>>>>
>>>> In the context of update-index, this is a one-time thing and leaking
>>>> is tolerable. But because it becomes a library function now, this leak
>>>> can become more serious, I think.
>>>>
>>>> The only other (indirect) caller is read_index_from() so probably not
>>>> bad most of the time (we read at the beginning of a command only).
>>>> sequencer.c may discard and re-read the index many times though,
>>>> leaking could be visible there.
>>>
>>> So is it enough to check if split_index->base->cache[] is shared with
>>> the_index.cache[] and then decide if discard_split_index(&the_index)
>>> should be called?
>>
>> It's likely shared though. We could un-share cache[] by duplicating
>> index entries in the_index.cache[] if they point back to
>> split_index->base
>
> I have a patch for this. So don't have to do anything else in this
> area. I'll probably just pile my patch on top of your series, or post
> it once the series graduates to master.
Great, thanks!
^ permalink raw reply
* Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early
From: Andreas Schwab @ 2016-11-09 14:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Johannes Sixt, git
In-Reply-To: <alpine.DEB.2.20.1611091437280.72596@virtualbox>
On Nov 09 2016, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> The reason why we do not ignore kill errors is that we want to make sure
> that the script *actually ran*. Otherwise, the thing we need to test here
> does not necessarily get tested.
That can be tested by looking for the pid file.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* [PATCH] sequencer: shut up clang warning
From: Johannes Schindelin @ 2016-11-09 13:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Torsten Bögershausen, Lars Schneider,
Jeff King
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
When comparing a value of type `enum todo_command` with a value that is
outside the defined enum constants, clang greets the developer with this
warning:
comparison of constant 2 with expression of type
'const enum todo_command' is always true
While this is arguably true *iff* the value was never cast from a
free-form int, we should keep the cautious code in place.
To shut up clang, we simply introduce an otherwise pointless enum constant
and compare against that.
Noticed by Torsten Bögershausen.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/enum-warning-v1
Fetch-It-Via: git fetch https://github.com/dscho/git enum-warning-v1
Peff, if you would like me to include more of your commit message,
please let me know.
I will adjust the sequencer-i patches to keep the TODO_INVALID
constant.
sequencer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..f80e9c0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
enum todo_command {
TODO_PICK = 0,
- TODO_REVERT
+ TODO_REVERT,
+ TODO_INVALID
};
static const char *todo_command_strings[] = {
@@ -629,7 +630,7 @@ static const char *todo_command_strings[] = {
static const char *command_to_string(const enum todo_command command)
{
- if (command < ARRAY_SIZE(todo_command_strings))
+ if (command < TODO_INVALID)
return todo_command_strings[command];
die("Unknown command: %d", command);
}
base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
--
2.10.1.583.g721a9e0
^ 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