* [PATCH] pre-receive.sample: Mark executable
From: Anders Kaseorg @ 2016-10-28 18:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
For consistency with other hooks, allow the hook to be activated by
renaming pre-receive.sample to pre-receive without a separate step to
mark it executable.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
templates/hooks--pre-receive.sample | 0
1 file changed, 0 insertions(+), 0 deletions(-)
mode change 100644 => 100755 templates/hooks--pre-receive.sample
diff --git a/templates/hooks--pre-receive.sample b/templates/hooks--pre-receive.sample
old mode 100644
new mode 100755
--
2.10.1
^ permalink raw reply
* Re: [PATCH] attr: convert to new threadsafe API
From: Johannes Sixt @ 2016-10-28 18:16 UTC (permalink / raw)
To: Stefan Beller, gitster
Cc: git, bmwill, pclouds, Johannes.Schindelin, peff, simon
In-Reply-To: <20161027221550.14930-1-sbeller@google.com>
Am 28.10.2016 um 00:15 schrieb Stefan Beller:
> * use attr_start on Windows to dynamically initialize the Single Big Attr Mutex
I'm sorry, I didn't find time to test the patch on Windows. I won't be
back at my Windows box before Wednesday.
-- Hannes
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Junio C Hamano @ 2016-10-28 17:47 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jeff King, Git Mailing List, Lars Schneider,
Eric Wong
In-Reply-To: <CA+55aFwUEzfvWVSZfhBi85QaKWSo-gVMOk1BJFrR0ZsdCRHRsg@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Oct 28, 2016 at 9:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Setting FD_CLOEXEC with fcntl(2) may be racy, but in what way
>> it is broken to open(2) with O_CLOEXEC?
>
> Apparently windows doesn't even support it, at least not mingw. So
> you'll have to have some supprt for just explicitly closing things at
> execve anyway. And if you do that for windows, then what's the point
> of using O_CLOEXEC on Linux, and having two totally different paths
> that will just mean maintenance headache.
Ah, that's where your reaction comes from. If I understood Dscho
correctly, what he said was that I cannot set FD_CLOEXEC via fcntl()
emulation they have. It wasn't an objection to O_CLOEXEC. In fact,
since 05d1ed6148 ("mingw: ensure temporary file handles are not
inherited by child processes", 2016-08-22), we have been relying on
open(2) with O_CLOEXEC even on Windows (by emulating it with
O_NOINHERIT, which Windows has) on some codepaths.
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Linus Torvalds @ 2016-10-28 17:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Jeff King, Git Mailing List, Lars Schneider,
Eric Wong
In-Reply-To: <xmqqshrg1ksv.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 28, 2016 at 9:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Excuse me, but care to elaborate a bit more?
It just seems entirely pointless to change the games with O_xyz.
> Setting FD_CLOEXEC with fcntl(2) may be racy, but in what way
> it is broken to open(2) with O_CLOEXEC?
Apparently windows doesn't even support it, at least not mingw. So
you'll have to have some supprt for just explicitly closing things at
execve anyway. And if you do that for windows, then what's the point
of using O_CLOEXEC on Linux, and having two totally different paths
that will just mean maintenance headache.
In contrast, O_NOATIME isn't a maintenance problem, since it's purely
an optimization and has no semantic difference, so it not existing on
some platform is immaterial.
End result: leave O_NOATIME as it is (working), and just forget about O_CLOEXEC.
I have no actual idea about mingw support for this, but Johannes'
email certainly implies it's painful on Windows. Some googling also
shows people doing things like
#ifdef O_CLOEXEC
flags |= O_CLOEXEC;
#endif
or
#ifndef O_CLOEXEC
#define O_CLOEXEC 0
#endif
so O_CLOEXEC definitely isn't considered portable. Do you really want
to rely on it?
Linus
^ permalink raw reply
* Re: [PATCH] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-28 17:30 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, pclouds, Johannes.Schindelin, j6t, peff, simon
In-Reply-To: <xmqqinsc1jcg.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>> + if (check)
>> + return; /* already done */
>> check = git_attr_check_alloc();
>> while (*argv) {
>> struct git_attr *attr = git_attr(*argv);
>> git_attr_check_append(check, attr);
I thought you made git_attr() constructor unavailable, so
check_append() would just get a "const char *" instead?
>> argv++;
>> }
>> + struct git_attr_result *result = git_attr_result_alloc(check);
>
> This does not look like thread-safe.
>
> I could understand it if the calling convention were like this,
> though:
>
> if (git_attr_check_alloc(&check)) {
> while (*argv) {
> ... append ...
> }
> git_attr_check_finished_appending(&check);
> }
> result = result_alloc();
>
> In this variant, git_attr_check_alloc() is responsible for ensuring
> that the "check" is allocated only once just like _initl() is, and
> at the same time, it makes simultanous callers wait until the first
> caller who appends to the singleton check instance declares that it
> finished appending. The return value signals if you are the first
> caller (who is responsible for populating the check and for
> declaring the check is ready to use at the end of appending).
> Everybody else waits while the first caller is doing the if (...) {
> } thing, and then receives false, at which time everybody (including
> the first caller) goes on and allocating its own result and start
> making queries.
Having said that, how flexible does the "alloc then append" side of
API have to be in the envisioned set of callers? Is it expected
that it wouldn't be too hard to arrange them so that they have an
array of "const char *"s when they need to initialize/populate a
check? If that is the case, instead of the "alloc and have others
wait" illustrated above, it may be a lot simpler to give _initv()
variant to them and be done with it, i.e. the above sample caller
would become just:
git_attr_check_initv(&check, argv);
result = git_attr_result_alloc(&check);
would that be too limiting? The use of "alloc then append" pattern
in "git check-attr" done in your patch seems to fit the _initv()
well, and the "pathspec with attr match" that appears later in the
series also has all the attributes it needs to query available by
the time it wants to allocate and append to create an instance of
"check", I would think, so the _initv() might be sufficient as a
public API.
^ permalink raw reply
* Re: [RFC PATCH 0/5] recursively grep across submodules
From: Junio C Hamano @ 2016-10-28 17:21 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <20161028170213.GA72114@google.com>
Brandon Williams <bmwill@google.com> writes:
>> Just a few brief comments, before reading the patches carefully.
>>
>> * It is somewhat surprising that [1/5] is even needed (in other
>> words, I would have expected something like this to be already
>> there, and my knee-jerk reaction was "Heh, how does 'git status'
>> know how to show submodules that are and are not initialized
>> differently without this?"
>
> Yeah I was also surprised to find that this kind of functionality didn't
> already exist. Though I guess there are still many builtin's that don't
> play nice with submodules so maybe this kind of functionality just
> wasn't needed until now.
>
>> * It is somewhat surprising that [4/5] does not even use the
>> previous ls-files to find out the paths.
>
> The first attempt I made at this series used ls-files to produce a list
> of files which was then fed to the grep machinery. The problem I found
> with this approach was when I started moving to work on grepping
> history, at that point it seemed to make more sense to have a process
> for each submodule.
Yup, that makes sense to me.
^ permalink raw reply
* Re: [PATCH] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-28 17:20 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, pclouds, Johannes.Schindelin, j6t, peff, simon
In-Reply-To: <20161027221550.14930-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> +* Prepare a `struct git_attr_check` using `git_attr_check_initl()`
> function, enumerating the names of attributes whose values you are
> interested in, terminated with a NULL pointer. Alternatively, an
> - empty `struct git_attr_check` can be prepared by calling
> - `git_attr_check_alloc()` function and then attributes you want to
> - ask about can be added to it with `git_attr_check_append()`
> - function.
> -
> -* Call `git_check_attr()` to check the attributes for the path.
> -
> -* Inspect `git_attr_check` structure to see how each of the
> - attribute in the array is defined for the path.
> -
> + empty `struct git_attr_check` as allocated by git_attr_check_alloc()
Need to drop "as allocated by git_attr_check_alloc()" here.
> + can be prepared by calling `git_attr_check_alloc()` function and
> + then attributes you want to ask about can be added to it with
> + `git_attr_check_append()` function.
> + Both ways with `git_attr_check_initl()` as well as the
> + alloc and append route are thread safe, i.e. you can call it
> + from different threads at the same time; when check determines
> + the initialization is still needed, the threads will use a
> + single global mutex to perform the initialization just once, the
> + others will wait on the the thread to actually perform the
> + initialization.
I have some comments on the example in the doc on the "alloc-append"
side. _initl() side looks OK.
> + static struct git_attr_check *check;
> + git_attr_check_initl(&check, "crlf", "ident", NULL);
OK.
> const char *path;
> + struct git_attr_result result[2];
>
> + git_check_attr(path, check, result);
OK. The above two may be easier to understand if they were a single
example, though.
> +. Act on `result.value[]`:
>
> ------------
> - const char *value = check->check[0].value;
> + const char *value = result.value[0];
OK.
> @@ -123,12 +135,15 @@ the first step in the above would be different.
> static struct git_attr_check *check;
> static void setup_check(const char **argv)
> {
> + if (check)
> + return; /* already done */
> check = git_attr_check_alloc();
> while (*argv) {
> struct git_attr *attr = git_attr(*argv);
> git_attr_check_append(check, attr);
> argv++;
> }
> + struct git_attr_result *result = git_attr_result_alloc(check);
This does not look like thread-safe.
I could understand it if the calling convention were like this,
though:
if (git_attr_check_alloc(&check)) {
while (*argv) {
... append ...
}
git_attr_check_finished_appending(&check);
}
result = result_alloc();
In this variant, git_attr_check_alloc() is responsible for ensuring
that the "check" is allocated only once just like _initl() is, and
at the same time, it makes simultanous callers wait until the first
caller who appends to the singleton check instance declares that it
finished appending. The return value signals if you are the first
caller (who is responsible for populating the check and for
declaring the check is ready to use at the end of appending).
Everybody else waits while the first caller is doing the if (...) {
} thing, and then receives false, at which time everybody (including
the first caller) goes on and allocating its own result and start
making queries.
> +* Setup a local variables for the question
> + `struct git_attr_check` as well as a pointer where the result
> + `struct git_attr_result` will be stored. Both should be initialized
> + to NULL.
> +
> +------------
> + struct git_attr_check *check = NULL;
> + struct git_attr_result *result = NULL;
> +------------
> +
> +* Call `git_all_attrs()`.
>
> +------------
> + git_all_attrs(full_path, &check, &result);
> +------------
OK.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 0/5] recursively grep across submodules
From: Brandon Williams @ 2016-10-28 17:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqk2ct4bmr.fsf@gitster.mtv.corp.google.com>
On 10/27, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > As for the rest of the series, it should be ready for review or comments.
>
> Just a few brief comments, before reading the patches carefully.
>
> * It is somewhat surprising that [1/5] is even needed (in other
> words, I would have expected something like this to be already
> there, and my knee-jerk reaction was "Heh, how does 'git status'
> know how to show submodules that are and are not initialized
> differently without this?"
Yeah I was also surprised to find that this kind of functionality didn't
already exist. Though I guess there are still many builtin's that don't
play nice with submodules so maybe this kind of functionality just
wasn't needed until now.
> * It is somewhat surprising that [4/5] does not even use the
> previous ls-files to find out the paths.
The first attempt I made at this series used ls-files to produce a list
of files which was then fed to the grep machinery. The problem I found
with this approach was when I started moving to work on grepping
history, at that point it seemed to make more sense to have a process
for each submodule.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Junio C Hamano @ 2016-10-28 16:48 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jeff King, Git Mailing List, Lars Schneider,
Eric Wong
In-Reply-To: <CA+55aFw93vkraxBvFCXFSYJqn836tXW+OCOFuToN+HaxTcJ7cg@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Oct 28, 2016 at 4:11 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>
>> You guys. I mean: You guys! You sure make my life hard. A brief look at
>> mingw.h could have answered your implicit question:
>
> So here's what you guys should do:
>
> - leave O_NOATIME damn well alone. It works. It has worked for 10+
> years. Stop arguing against it, people who do.
>
> - get rid of all O_CLOEXEC games. They don't work. If you want to
> close file descriptors at execve(), you - gasp - close the file
> descriptor before doing an execve.
>
> So O_CLOEXEC or FD_CLOEXEC is broken.
>
> DO NOT BREAK O_NOATIME JUST TO ADD COMPLETELY NEW BREAKAGE.
Excuse me, but care to elaborate a bit more?
Did I botch the way I ask fcntl(2) to set O_NOATIME in *1*? Its
follow-up in *2* is optional and as peff said in *3*, I do not think
we mind dropping that step and keeping O_NOATIME. It is not that
much code to keep.
Setting FD_CLOEXEC with fcntl(2) may be racy, but in what way
it is broken to open(2) with O_CLOEXEC?
If we want to close file descriptors we opened at execve() time
ourselves, we'd need to somehow keep track of which file descriptors
are meant to be closed upon execve() time, and one way to mark that
may be to use O_CLOEXEC, but once we mark a file descriptor with the
bit, execve() would take care of "closing" it for us---wouldn't that
be the whole point of that bit?
*1* http://public-inbox.org/git/xmqqh97w38gj.fsf@gitster.mtv.corp.google.com
*2* http://public-inbox.org/git/xmqqd1ik38f4.fsf@gitster.mtv.corp.google.com
*3* http://public-inbox.org/git/20161028075104.la24zydnr3ogb6qv@sigill.intra.peff.net
^ permalink raw reply
* Re: [RFC PATCH 0/5] recursively grep across submodules
From: Philip Oakley @ 2016-10-28 15:06 UTC (permalink / raw)
To: Junio C Hamano, Stefan Beller; +Cc: Brandon Williams, Git List
In-Reply-To: <xmqq1sz1425w.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
>
> I hate it when people become overly defensive and start making
> excuses when given harmless observations.
>
Hi Junio,
It can sometimes be difficult for readers to appreciate which way comments
are meant to be interpreted, especially as one cannot usually 'see' the
issue being raised with one's personal work, no matter who writes them. I
too have to supportively review the work of others (as a volunteer), who
then don't always respond or understand, as hoped, and it can be
frustrating.
It can be very hard to write a reasonable write up that gets the balance
between being on the one hand patronising (e.g. over-explained) and on the
other too terse, and yet still not be too nuanced that the points are
missed. The responder has the similar problem, especially if they have
misunderstood the comment, and then end up just end up digging the hole
deeper by over-explaining their position. Extricating the discussion from
the trap can be tricky.
Thank you for your reviews.
--
Philip
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Philip Oakley @ 2016-10-28 13:01 UTC (permalink / raw)
To: Jacob Keller, Johannes Sixt
Cc: Stefan Beller, Junio C Hamano, Johannes Schindelin, git,
Simon Ruderich, Jeff King
In-Reply-To: <2ddca5e3-3c4d-b555-4309-a180ceed581e@kdbg.org>
From: "Johannes Sixt" <j6t@kdbg.org>
>
> One point is that the DCLP idiom must be implemented correctly. There are
> solutions, of course, and when the initialization is over, we have a
> miniscule overhead at each pthread_mutex_lock call.
>
I had to look up DCLP ( = Double Checked Locking Patterns), and found a good
write up on the issues..
http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf "C++ and the
Perils of Double-Checked Locking", which include 'C' issues, and
multi-thread, multi-processor issues. Not an easy issue when fighting
optimisers..
--
Philip
^ permalink raw reply
* Re: feature request
From: Philip Oakley @ 2016-10-28 12:54 UTC (permalink / raw)
To: David Lang, John Rood; +Cc: Stefan Beller, Git List
In-Reply-To: <alpine.DEB.2.02.1610271623260.4123@nftneq.ynat.uz>
From: "David Lang" <david@lang.hm>
> On Thu, 27 Oct 2016, John Rood wrote:
>
>> Thanks, I think changing the default for windows is a good idea.
>
> notepad doesn't work well with unix line endings, wordpad handles the
> files much more cleanly.
>
> David Lang
>
Notepad++ does work well, but isn't a standard part of Windows.
[core]
editor = 'C:/Program
Files/Notepad++/notepad++.exe' -multiInst -notabbar -nosession -noplugin
.. is one of the standard StackOverflow recipes.
--
Philip
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Linus Torvalds @ 2016-10-28 16:13 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Jeff King, Git Mailing List, Lars Schneider,
Eric Wong
In-Reply-To: <alpine.DEB.2.20.1610281306320.3264@virtualbox>
On Fri, Oct 28, 2016 at 4:11 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> You guys. I mean: You guys! You sure make my life hard. A brief look at
> mingw.h could have answered your implicit question:
So here's what you guys should do:
- leave O_NOATIME damn well alone. It works. It has worked for 10+
years. Stop arguing against it, people who do.
- get rid of all O_CLOEXEC games. They don't work. If you want to
close file descriptors at execve(), you - gasp - close the file
descriptor before doing an execve.
So O_CLOEXEC or FD_CLOEXEC is broken.
DO NOT BREAK O_NOATIME JUST TO ADD COMPLETELY NEW BREAKAGE.
Linus
^ permalink raw reply
* Re: [PATCH] Fix typo in 2.11.0 RelNotes
From: Henrik Ahlgren @ 2016-10-28 15:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqoa2439sp.fsf@gitster.mtv.corp.google.com>
On Fri, 2016-10-28 at 06:03 -0700, Junio C Hamano wrote:
> If the original text were like the following, would it have been
> clear enough that prevented you from sending your patch?
>
> * An empty string used as a pathspec element has always meant
> 'everything matches', but it is too easy to write a script that
> finds a path to remove in $path and run 'git rm "$paht"' by
> mistake (when the user meant to give "$path"), which ends up
> removing everything. This release starts warning about the
> use of an empty string that is used for 'everything matches' and
> asks users to use a more explicit '.' for that instead.
Oops. Yes, but not sure if it really needs to be clear enough even for
idiots like me. :-) Sorry.
^ permalink raw reply
* Re: [PATCH] Documenation: fmt-merge-msg: fix markup in example
From: Junio C Hamano @ 2016-10-28 15:15 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Christ, git
In-Reply-To: <20161028110820.a46ttxjicq2k5xdk@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Oct 28, 2016 at 12:01:26PM +0200, Stefan Christ wrote:
>
>> diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
>> index 6526b17..44892c4 100644
>> --- a/Documentation/git-fmt-merge-msg.txt
>> +++ b/Documentation/git-fmt-merge-msg.txt
>> @@ -60,10 +60,10 @@ merge.summary::
>> EXAMPLE
>> -------
>>
>> ---
>> +---------
>> $ git fetch origin master
>> $ git fmt-merge-msg --log <$GIT_DIR/FETCH_HEAD
>> ---
>> +---------
>
> Thanks. Asciidoc generally requires at least 4 delimiter characters to
> open a delimited block (including a ListingBlock, which is what we want
> here). There is one exception, "--", which is a generic OpenBlock, which
> is just used for grouping, and not any special syntactic meaning (so
> that's why this _didn't_ render the "--", but did render the contents
> without line breaks).
>
> So looks good, modulo the typo in the subject that somebody else pointed
> out.
Thanks, both. I queued with a bit more enhanced log message while
fixing the typo.
-- >8 --
From: Stefan Christ <contact@stefanchrist.eu>
Date: Fri, 28 Oct 2016 12:01:26 +0200
Subject: [PATCH] Documentation/fmt-merge-msg: fix markup in example
Use at least 4 delimiting dashes that are required for
ListingBlock to get this block rendered as verbatim text.
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-fmt-merge-msg.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index 6526b178e8..44892c447e 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -60,10 +60,10 @@ merge.summary::
EXAMPLE
-------
---
+---------
$ git fetch origin master
$ git fmt-merge-msg --log <$GIT_DIR/FETCH_HEAD
---
+---------
Print a log message describing a merge of the "master" branch from
the "origin" remote.
--
2.10.1-791-g404733b9cf
^ permalink raw reply related
* [PATCH v2] Documentation: fmt-merge-msg: fix markup in example
From: Stefan Christ @ 2016-10-28 15:07 UTC (permalink / raw)
To: git; +Cc: rpjday, peff
In-Reply-To: <alpine.LFD.2.20.1610280623400.524@localhost.localdomain>
The example was not rendered as verbatim text. Fix it.
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
v2: fix misspelling in subject
Hi,
thanks to rday for spotting the typo in the subject line (I missed to
autoenable the spell checking in vim) and thanks to Peff for the asciidoc
explanation.
Kind regards,
Stefan Christ
---
Documentation/git-fmt-merge-msg.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index 6526b17..44892c4 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -60,10 +60,10 @@ merge.summary::
EXAMPLE
-------
---
+---------
$ git fetch origin master
$ git fmt-merge-msg --log <$GIT_DIR/FETCH_HEAD
---
+---------
Print a log message describing a merge of the "master" branch from
the "origin" remote.
--
2.7.3
^ permalink raw reply related
* Is the entire working copy “at one branch”?
From: Stefan Monov @ 2016-10-28 15:01 UTC (permalink / raw)
To: git
This is a very basic question but maybe I just don't know how to
phrase my query well enough to find an answer.
Let me begin with a SVN example. SVN doesn't have a "current branch"
as a property of a working copy, so I'll give the example with
revisions instead. Consider a working copy with the dirs `foo` and
`bar` in it. In SVN one can `update` foo to an earlier version, but
keep `bar` at `HEAD`.
Is it possible in an analogical git working copy to checkout one
branch in dir `foo` without changing the branch checked out in `bar`?
If not, why not?
TIA,
Stefan Monov
^ permalink raw reply
* Git crash course for SVN users - which of the 2?
From: Stefan Monov @ 2016-10-28 14:42 UTC (permalink / raw)
To: git
I googled and found these two official-looking results:
https://git.wiki.kernel.org/index.php/GitSvnCrashCourse
https://git-scm.com/course/svn.html
Which of the two should I use? And I'd ask for the other one to be
deleted so there's no such further confusion for other user.
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Junio C Hamano @ 2016-10-28 13:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Git Mailing List, Lars Schneider, Eric Wong,
Johannes Schindelin
In-Reply-To: <xmqqh97w38gj.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Hmph. This may not fly well in practice, though.
>
> We can flip the order around, and then it becomes simpler to later
> drop atime support. The first step goes like this.
And the second step would be like this.
-- >8 --
Subject: [PATCH] sha1_file: stop opening files with O_NOATIME
When we open object files, we try to do so with O_NOATIME.
This dates back to 144bde78e9 (Use O_NOATIME when opening
the sha1 files., 2005-04-23), which is an optimization to
avoid creating a bunch of dirty inodes when we're accessing
many objects. But a few things have changed since then:
1. In June 2005, git learned about packfiles, which means
we would do a lot fewer atime updates (rather than one
per object access, we'd generally get one per packfile).
2. In late 2006, Linux learned about "relatime", which is
generally the default on modern installs. So
performance around atimes updates is a non-issue there
these days.
All the world isn't Linux, but as it turns out, Linux
is the only platform to implement O_NOATIME in the
first place.
So it's very unlikely that this code is helping anybody
these days.
Helped-by: Jeff King <peff@peff.net>
[jc: took idea and log message from peff]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache.h | 2 +-
sha1_file.c | 21 ---------------------
2 files changed, 1 insertion(+), 22 deletions(-)
diff --git a/cache.h b/cache.h
index 6f1c21a352..f440d3fd1e 100644
--- a/cache.h
+++ b/cache.h
@@ -1123,7 +1123,7 @@ extern int hash_sha1_file_literally(const void *buf, unsigned long len, const ch
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
extern int git_open_cloexec(const char *name, int flags);
-extern int git_open(const char *name);
+#define git_open(name) git_open_cloexec(name, O_RDONLY)
extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
diff --git a/sha1_file.c b/sha1_file.c
index dfbf398183..25d9359402 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -27,14 +27,6 @@
#include "list.h"
#include "mergesort.h"
-#ifndef O_NOATIME
-#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
-#define O_NOATIME 01000000
-#else
-#define O_NOATIME 0
-#endif
-#endif
-
#define SZ_FMT PRIuMAX
static inline uintmax_t sz_fmt(size_t s) { return s; }
@@ -1572,19 +1564,6 @@ int git_open_cloexec(const char *name, int flags)
return fd;
}
-int git_open(const char *name)
-{
- static int noatime = O_NOATIME;
- int fd = git_open_cloexec(name, O_RDONLY);
-
- if (0 <= fd && (noatime & O_NOATIME)) {
- int flags = fcntl(fd, F_GETFL);
- if (fcntl(fd, F_SETFL, flags | noatime))
- noatime = 0;
- }
- return fd;
-}
-
static int stat_sha1_file(const unsigned char *sha1, struct stat *st)
{
struct alternate_object_database *alt;
--
2.10.1-791-g404733b9cf
^ permalink raw reply related
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Junio C Hamano @ 2016-10-28 13:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Git Mailing List, Lars Schneider, Eric Wong,
Johannes Schindelin
In-Reply-To: <xmqq60od42s0.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Hmph. This may not fly well in practice, though.
We can flip the order around, and then it becomes simpler to later
drop atime support. The first step goes like this.
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 28 Oct 2016 06:23:07 -0700
Subject: [PATCH] git_open(): untangle possible NOATIME and CLOEXEC interactions
The way we structured the fallback/retry mechanism for opening with
O_NOATIME and O_CLOEXEC meant that if we failed due to lack of
support to open the file with O_NOATIME option (i.e. EINVAL), we
would still try to drop O_CLOEXEC first and retry, and then drop
O_NOATIME. A platform on which O_NOATIME is defined in the header
without support from the kernel wouldn't have a chance to open with
O_CLOEXEC option due to this code structure.
Arguably, O_CLOEXEC is more important than O_NOATIME, as the latter
is mostly about performance, while the former can affect correctness.
Instead use O_CLOEXEC to open the file, and then use fcntl(2) to set
O_NOATIME on the resulting file descriptor. open(2) itself does not
cause atime to be updated according to Linus [*1*].
The helper to do the former can be usable in the codepath in
ce_compare_data() that was recently added to open a file descriptor
with O_CLOEXEC; use it while we are at it.
*1* <CA+55aFw83E+zOd+z5h-CA-3NhrLjVr-anL6pubrSWttYx3zu8g@mail.gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cache.h | 1 +
read-cache.c | 9 +--------
sha1_file.c | 39 +++++++++++++++++++--------------------
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/cache.h b/cache.h
index a902ca1f8e..6f1c21a352 100644
--- a/cache.h
+++ b/cache.h
@@ -1122,6 +1122,7 @@ extern int write_sha1_file(const void *buf, unsigned long len, const char *type,
extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int git_open_cloexec(const char *name, int flags);
extern int git_open(const char *name);
extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
diff --git a/read-cache.c b/read-cache.c
index db5d910642..c27d3e240b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,14 +156,7 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)
static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
{
int match = -1;
- static int cloexec = O_CLOEXEC;
- int fd = open(ce->name, O_RDONLY | cloexec);
-
- if ((cloexec & O_CLOEXEC) && fd < 0 && errno == EINVAL) {
- /* Try again w/o O_CLOEXEC: the kernel might not support it */
- cloexec &= ~O_CLOEXEC;
- fd = open(ce->name, O_RDONLY | cloexec);
- }
+ int fd = git_open_cloexec(ce->name, O_RDONLY);
if (fd >= 0) {
unsigned char sha1[20];
diff --git a/sha1_file.c b/sha1_file.c
index 09045df1dc..dfbf398183 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1559,31 +1559,30 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
return hashcmp(sha1, real_sha1) ? -1 : 0;
}
-int git_open(const char *name)
+int git_open_cloexec(const char *name, int flags)
{
- static int sha1_file_open_flag = O_NOATIME | O_CLOEXEC;
-
- for (;;) {
- int fd;
-
- errno = 0;
- fd = open(name, O_RDONLY | sha1_file_open_flag);
- if (fd >= 0)
- return fd;
+ static int cloexec = O_CLOEXEC;
+ int fd = open(name, flags | cloexec);
+ if ((cloexec & O_CLOEXEC) && fd < 0 && errno == EINVAL) {
/* Try again w/o O_CLOEXEC: the kernel might not support it */
- if ((sha1_file_open_flag & O_CLOEXEC) && errno == EINVAL) {
- sha1_file_open_flag &= ~O_CLOEXEC;
- continue;
- }
+ cloexec &= ~O_CLOEXEC;
+ fd = open(name, flags | cloexec);
+ }
+ return fd;
+}
- /* Might the failure be due to O_NOATIME? */
- if (errno != ENOENT && (sha1_file_open_flag & O_NOATIME)) {
- sha1_file_open_flag &= ~O_NOATIME;
- continue;
- }
- return -1;
+int git_open(const char *name)
+{
+ static int noatime = O_NOATIME;
+ int fd = git_open_cloexec(name, O_RDONLY);
+
+ if (0 <= fd && (noatime & O_NOATIME)) {
+ int flags = fcntl(fd, F_GETFL);
+ if (fcntl(fd, F_SETFL, flags | noatime))
+ noatime = 0;
}
+ return fd;
}
static int stat_sha1_file(const unsigned char *sha1, struct stat *st)
--
2.10.1-791-g404733b9cf
^ permalink raw reply related
* Re: [PATCH] Fix typo in 2.11.0 RelNotes
From: Junio C Hamano @ 2016-10-28 13:03 UTC (permalink / raw)
To: Henrik Ahlgren; +Cc: git
In-Reply-To: <1477651639-11529-1-git-send-email-pablo@seestieto.com>
Henrik Ahlgren <pablo@seestieto.com> writes:
> Signed-off-by: Henrik Ahlgren <pablo@seestieto.com>
> ---
> Documentation/RelNotes/2.11.0.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/RelNotes/2.11.0.txt b/Documentation/RelNotes/2.11.0.txt
> index 3590620..1d3a07d 100644
> --- a/Documentation/RelNotes/2.11.0.txt
> +++ b/Documentation/RelNotes/2.11.0.txt
> @@ -5,7 +5,7 @@ Backward compatibility notes.
>
> * An empty string used as a pathspec element has always meant
> 'everything matches', but it is too easy to write a script that
> - finds a path to remove in $path and run 'git rm "$paht"', which
> + finds a path to remove in $path and run 'git rm "$path"', which
> ends up removing everything. This release starts warning about the
> use of an empty string that is used for 'everything matches' and
> asks users to use a more explicit '.' for that instead.
What you spotted is certainly a typo, but it is a deliberate one
that must not be fixed like this. "..., but it is too easy to ..."
is illustrating a scenario in which an empty string is accidentally
given to "git rm" as a pathspec by mistake, and the example it uses
is for the user to prepare a path to be removed in variable $path,
and referring to it as its typoed $paht by mistake. Fixing that typo
in this paragraph defeats the whole point of the example.
But the fact that you spotted the typo (which is good; we want the
deliberate typo in the example to be clearly visible) and thought
that the writer of the paragraph must have meant a non-typoed
version there (which is bad) indicates that the paragraph needs
improvement to save readers from making the same mis-reading as you
did.
If the original text were like the following, would it have been
clear enough that prevented you from sending your patch?
* An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"' by
mistake (when the user meant to give "$path"), which ends up
removing everything. This release starts warning about the
use of an empty string that is used for 'everything matches' and
asks users to use a more explicit '.' for that instead.
Thanks.
^ permalink raw reply
* Re: [PATCH] valgrind: support test helpers
From: Junio C Hamano @ 2016-10-28 12:50 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Duy Nguyen, Johannes Schindelin
In-Reply-To: <71c3d13a-fa29-75d3-50ac-81978c08f552@web.de>
René Scharfe <l.s.r@web.de> writes:
> Tests run with --valgrind call git commands through a wrapper script
> that invokes valgrind on them. This script (valgrind.sh) is in turn
> invoked through symlinks created for each command in t/valgrind/bin/.
>
> Since e6e7530d (test helpers: move test-* to t/helper/ subdirectory)
> these symlinks have been broken for test helpers -- they point to the
> old locations in the root of the build directory. Fix that by teaching
> the code for creating the links about the new location of the binaries,
> and do the same in the wrapper script to allow it to find its payload.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
Hmph. I somehow thought this was supposed to have been fixed by
503e224180 ("t/test-lib.sh: fix running tests with --valgrind",
2016-07-11) already.
> t/test-lib.sh | 9 ++++++++-
> t/valgrind/valgrind.sh | 12 ++++++++++--
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index b859db6..a724181 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -809,7 +809,14 @@ then
> return;
>
> base=$(basename "$1")
> - symlink_target=$GIT_BUILD_DIR/$base
> + case "$base" in
> + test-*)
> + symlink_target="$GIT_BUILD_DIR/t/helper/$base"
> + ;;
> + *)
> + symlink_target="$GIT_BUILD_DIR/$base"
> + ;;
> + esac
> # do not override scripts
> if test -x "$symlink_target" &&
> test ! -d "$symlink_target" &&
> diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
> index 4215303..669ebaf 100755
> --- a/t/valgrind/valgrind.sh
> +++ b/t/valgrind/valgrind.sh
> @@ -1,11 +1,19 @@
> #!/bin/sh
>
> base=$(basename "$0")
> +case "$base" in
> +test-*)
> + program="$GIT_VALGRIND/../../t/helper/$base"
> + ;;
> +*)
> + program="$GIT_VALGRIND/../../$base"
> + ;;
> +esac
>
> TOOL_OPTIONS='--leak-check=no'
>
> test -z "$GIT_VALGRIND_ENABLED" &&
> -exec "$GIT_VALGRIND"/../../"$base" "$@"
> +exec "$program" "$@"
>
> case "$GIT_VALGRIND_MODE" in
> memcheck-fast)
> @@ -29,4 +37,4 @@ exec valgrind -q --error-exitcode=126 \
> --log-fd=4 \
> --input-fd=4 \
> $GIT_VALGRIND_OPTIONS \
> - "$GIT_VALGRIND"/../../"$base" "$@"
> + "$program" "$@"
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Schindelin @ 2016-10-28 11:58 UTC (permalink / raw)
To: Jacob Keller
Cc: Johannes Sixt, Stefan Beller, Junio C Hamano, git@vger.kernel.org,
Simon Ruderich, Jeff King
In-Reply-To: <CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com>
Hi Jake,
On Thu, 27 Oct 2016, Jacob Keller wrote:
> I agree with Stefan that there isn't really a great place to put a
> dynamic initialization.
Ummm. Wait. What???
https://github.com/git/git/blob/v2.10.1/common-main.c#L25-L41
Ciao,
Johannes
^ permalink raw reply
* [PATCH] Fix typo in 2.11.0 RelNotes
From: Henrik Ahlgren @ 2016-10-28 10:47 UTC (permalink / raw)
To: git; +Cc: Henrik Ahlgren
Signed-off-by: Henrik Ahlgren <pablo@seestieto.com>
---
Documentation/RelNotes/2.11.0.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/RelNotes/2.11.0.txt b/Documentation/RelNotes/2.11.0.txt
index 3590620..1d3a07d 100644
--- a/Documentation/RelNotes/2.11.0.txt
+++ b/Documentation/RelNotes/2.11.0.txt
@@ -5,7 +5,7 @@ Backward compatibility notes.
* An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
- finds a path to remove in $path and run 'git rm "$paht"', which
+ finds a path to remove in $path and run 'git rm "$path"', which
ends up removing everything. This release starts warning about the
use of an empty string that is used for 'everything matches' and
asks users to use a more explicit '.' for that instead.
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Johannes Schindelin @ 2016-10-28 11:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Jeff King, Git Mailing List, Lars Schneider,
Eric Wong
In-Reply-To: <xmqq60od42s0.fsf@gitster.mtv.corp.google.com>
Hi,
On Thu, 27 Oct 2016, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Linus Torvalds <torvalds@linux-foundation.org> writes:
> >
> >> On Thu, Oct 27, 2016 at 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >>>
> >>> Would the best endgame shape for this function be to open with
> >>> O_NOATIME (and retry without), and then add CLOEXEC with fcntl(2)
> >>> but ignoring an error from it, I guess? That would be the closest
> >>> to what we historically had, I would think.
> >>
> >> I think that's the best model.
> >
> > OK, so perhaps like this.
>
> Hmph. This may not fly well in practice, though.
>
> To Unix folks, CLOEXEC is not a huge correctness issue. A child
> process may hold onto an open file descriptor a bit longer than the
> lifetime of the parent but as long as the child eventually exits,
> nothing is affected. Over there, things are different. The parent
> cannot even rename(2) or unlink(2) a file it created and closed
> while the child is still holding the file descriptor open and the
> lack of CLOEXEC will make the parent fail. I do not know how well
> fcntl(2) emulation works on Windows, but I would not be surprised
> if J6t or Dscho comes back and says that FD_CLOEXEC given to F_SETFD
> would not work while O_CLOEXEC given to open(2) does.
You guys. I mean: You guys! You sure make my life hard. A brief look at
mingw.h could have answered your implicit question:
static inline int fcntl(int fd, int cmd, ...)
{
if (cmd == F_GETFD || cmd == F_SETFD)
return 0;
errno = EINVAL;
return -1;
}
So while you discuss in your Linux Ivory Tower how to optimize Git for
Linux, and Linux only, I'll have to drop everything else and spend the
rest of my Friday trying to find a way to adjust a file handle
*immediately after opening it with undesired flags* (when it could have
been opened with the desired flags, as suggested, to begin with).
Ciao,
Johannes
^ 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