* 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
* Re: [PATCH] Documenation: fmt-merge-msg: fix markup in example
From: Jeff King @ 2016-10-28 11:08 UTC (permalink / raw)
To: Stefan Christ; +Cc: git
In-Reply-To: <1477648886-12096-1-git-send-email-contact@stefanchrist.eu>
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.
-Peff
^ permalink raw reply
* Re: [PATCH] Documenation: fmt-merge-msg: fix markup in example
From: Robert P. J. Day @ 2016-10-28 10:24 UTC (permalink / raw)
To: Stefan Christ; +Cc: git
In-Reply-To: <1477648886-12096-1-git-send-email-contact@stefanchrist.eu>
"Documenation"?
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* [PATCH] Documenation: fmt-merge-msg: fix markup in example
From: Stefan Christ @ 2016-10-28 10:01 UTC (permalink / raw)
To: git
The example was not rendered as verbatim text. Fix it.
Signed-off-by: Stefan Christ <contact@stefanchrist.eu>
---
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
* Re: Expanding Includes in .gitignore
From: Aaron Pelly @ 2016-10-28 9:32 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git
In-Reply-To: <xmqqwpgt2ng2.fsf@gitster.mtv.corp.google.com>
On 28/10/16 15:54, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> However, as I said elsewhere, I'm not convinced this feature is all that
>> helpful for in-repository .gitignore files, and I think it does
>> introduce compatibility complications. People with older git will not
>> respect your .gitignore.d files. Whereas $GIT_DIR/info is purely a local
>> matter.
>
> As I do not see the point of making in-tree .gitignore to a forest
> of .gitignore.d/ at all, compatibility complications is not worth
> even thinking about, I would have to say.
Well; that saves some work. :)
I do not suggesting making this mandatory. I think it adds value and it
is a common and understood mechanism. But, if it is abhorrent, consider:
There is precedent for including files in git-config. This could be
extended to ignore files. The code is not similar, but the concept is. I
could live with it.
Or how about a new githook that can intelligently create or return the
details? This would be my least favourite option unless it was
configured in an obvious place.
Finally, if this is a bad-idea, as I asked in the beginning, I will
consider the equine expired, cease flagellation and apologise for the noise.
^ permalink raw reply
* Re: [PATCH] attr: convert to new threadsafe API
From: Johannes Schindelin @ 2016-10-28 8:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, bmwill, pclouds, j6t, peff, simon
In-Reply-To: <20161027221550.14930-1-sbeller@google.com>
Hi Stefan,
On Thu, 27 Oct 2016, Stefan Beller wrote:
> * use attr_start on Windows to dynamically initialize the Single Big Attr Mutex
I would have preferred that call in common-main.c, but whatevs...
Thanks you for fixing the bug,
Dscho
^ permalink raw reply
* Re: [PATCH] valgrind: support test helpers
From: Johannes Schindelin @ 2016-10-28 8:51 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Duy Nguyen, Junio C Hamano
In-Reply-To: <71c3d13a-fa29-75d3-50ac-81978c08f552@web.de>
[-- Attachment #1: Type: text/plain, Size: 764 bytes --]
Hi,
On Fri, 28 Oct 2016, René Scharfe wrote:
> 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>
Apart from the missing accent ("é") in your SOB: ACK.
Ciao,
Dscho
^ permalink raw reply
* Re: feature request
From: Johannes Schindelin @ 2016-10-28 8:49 UTC (permalink / raw)
To: David Lang; +Cc: John Rood, Stefan Beller, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.02.1610271623260.4123@nftneq.ynat.uz>
Hi,
On Thu, 27 Oct 2016, David Lang wrote:
> 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.
That is why we have a `notepad` helper in Git for Windows that converts
line endings transparently before and after calling the real notepad.exe.
Ciao,
Johannes
^ permalink raw reply
* Re: Expanding Includes in .gitignore
From: Jeff King @ 2016-10-28 8:10 UTC (permalink / raw)
To: Aaron Pelly; +Cc: git
In-Reply-To: <91e0f377-ecfd-ab0a-4f4b-8c0f762228aa@pelly.co>
On Fri, Oct 28, 2016 at 11:17:26AM +1300, Aaron Pelly wrote:
> On 28/10/16 10:55, Aaron Pelly wrote:
> > 2) I fetch a repo with a hostile ignore file. It includes files from
> > $GIT_DIR/test-data/ssl/private or some such. Change. Don't pay
> > attention. Commit. Push. Problems if my test data comes from production.
> >
> > Is this mitigated currently?
> >
> > Not that git should be an enabler, but surely it falls on the user of
> > untrusted software to ensure their own security?
>
> Balls, I meant $GIT_WORK_TREE not $GIT_DIR
I was going to ask what you meant by "currently" here, as we do not yet
have an include mechanism, and generally things in the repository are
fair game. But I guess you mean that there could be untracked files even
inside the repository.
I'm not too worried about that in general. You have to be careful of a
lot of things inside the repository, like running "make" on malicious
code. Adding "don't stick secret files inside the repository, even
untracked", does seem like another sensible precaution.
The main thing with malicious repositories is that basic inspection like
"git clone $remote && git log" should not execute arbitrary code, leak
information, etc.
-Peff
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Jeff King @ 2016-10-28 7:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Git Mailing List, Lars Schneider, Eric Wong,
Johannes Schindelin
In-Reply-To: <CA+55aFwfhFqV74s_O=GucycY9U19ysiACDqX=mK4Gf=eQ0coxQ@mail.gmail.com>
On Thu, Oct 27, 2016 at 03:38:59PM -0700, Linus Torvalds wrote:
> On Thu, Oct 27, 2016 at 3:24 AM, Jeff King <peff@peff.net> wrote:
> >
> > +cc Linus as the original author of 144bde78e9 in case there is
> > something subtle I'm missing, but this really just seems like it's
> > an outdated optimization.
>
> I'd *really* like to keep O_NOATIME if at all possible. It made a huge
> difference on older kernels, and I'm not convinced that relatime
> really fixes it as well as O_NOATIME.
>
> There are people who don't like relatime. And even if you do have
> relatime enabled, it will update atime once every day, so then this
> makes your filesystem have a storm of nasty inode writebacks if you
> haven't touched that git repo in a while.
The existence of "relatime" is only half the story of its outdatedness.
The other half is packfiles, so that we are paying atime only once per
packfile, not once per object (technically once per mmap(), so on a
32-bit system with large packfiles, it would be multiple, depending on
your window size).
So I'm not convinced that "storm" is really the right word in a modern
context. The atime updates due to object accesses are probably smaller
than those from all the other read() calls being done on non-object
files (like config, refs, etc).
That being said, if you really care, it's not that much code to keep.
-Peff
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: Duy Nguyen @ 2016-10-28 6:38 UTC (permalink / raw)
To: Eric Wong; +Cc: Johannes Schindelin, Junio C Hamano, Git Mailing List
In-Reply-To: <20161027204129.GA21619@starla>
On Fri, Oct 28, 2016 at 3:41 AM, Eric Wong <e@80x24.org> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> I know you are a fan of testing things thoroughly in the test suite, but I
>> have to say that it is getting out of hand, in particular due to our
>> over-use of shell script idioms (which really only run fast on Linux, not
>> a good idea for a portable software).
>
> How much effort would it take to optimize a /bin/sh?
>
> Would replacing uses of fork+execve posix_spawn be fast and
> portable enough?
>
> Even on Linux, performance sucks for me. I've been hoping dash
> can use posix_spawn (or using vfork directly) to see if that can
> help things.
Busybox hush uses vfork.on no-mmu builds. You could try it out. I
don't know how complete it is though.
--
Duy
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Jacob Keller @ 2016-10-28 6:11 UTC (permalink / raw)
To: Johannes Sixt
Cc: Stefan Beller, Junio C Hamano, Johannes Schindelin,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <2ddca5e3-3c4d-b555-4309-a180ceed581e@kdbg.org>
On Thu, Oct 27, 2016 at 10:55 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> 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.
>
Right, this I understood, but appeared to be solved.
> The main point is that the initialization has to solve a chicken-and-egg
> problem: After we have found an uninitialized critical section, we have to
> have mutual exclusion for the initialization. We need another critical
> section for this, but we cannot have one that is initialized. For this
> reason, the solution uses a different kind of mutual exclusion primitive,
> which is more akin to POSIX semaphores and works across processes. In the
> patch proposed by Stefan, a *session-wide* mutex is used. That means that
> all concurrent git invocations in a user's session synchronize their
> initialization of critical section objects.
>
Thank you for explaining this. Now I understand why this would be
considered a big issue, and potentially worth considering the
alternatives like attr_start. This was missing since I don't think any
of the rest of us knew (correct me if I am wrong) that the
synchronization would be global. For many cases it's probably not that
bad, but we do have a suitable explanation, and I think living with
"attr_start()" in the win32 initialization path which is something
Stefan suggested in a previous email would make the most sense then.
> That's just ridiculous. It's like waiting for a ... no, *the* ... battle
> ship just to get our bouncers in their position. We are talking milliseconds
> here, not nanoseconds.
>
Right. Thanks for filling in the missing details.
Regards,
Jake
> -- Hannes
>
^ permalink raw reply
* Re: [PATCH v15 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
From: Matthieu Moy @ 2016-10-28 6:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Pranit Bauva, Christian Couder, Alex Henrie, Antoine Delaite
In-Reply-To: <xmqqvawd7mnr.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Cc'ed those who touched either "git-bisect.sh" or "builtin/bisect-helper.c"
> in our relatively recent past.
>
> Does any of you (and others on the list) have time and inclination
> to review this series?
Unfortunately, I have essentially zero-bandwidth to do that in the near
future :-(.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] compat: Allow static initializer for pthreads on Windows
From: Johannes Sixt @ 2016-10-28 5:55 UTC (permalink / raw)
To: Jacob Keller
Cc: Stefan Beller, Junio C Hamano, Johannes Schindelin,
git@vger.kernel.org, Simon Ruderich, Jeff King
In-Reply-To: <CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com>
Am 27.10.2016 um 23:49 schrieb Jacob Keller:
> Ok, so I've been reading this thread. I don't understand your
> objections to emulating in this way.. Could you clearly spell out why
> you believe this solution isn't acceptable? So far all I've understood
> was "it's not critical sections" and "it penalizes Windows too much"
> but... If Windows cannot statically initialize a pthread mutex, then
> we *have* to dynamically initialize it somewhere. This solution adds a
> single check before each lock and is safe due to use of memory
> barriers. Yes, this will cost a tiny bit extra overhead for each use
> of "pthread_mutex_lock" but I fail to see how that is a huge
> penalty...
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.
The main point is that the initialization has to solve a chicken-and-egg
problem: After we have found an uninitialized critical section, we have
to have mutual exclusion for the initialization. We need another
critical section for this, but we cannot have one that is initialized.
For this reason, the solution uses a different kind of mutual exclusion
primitive, which is more akin to POSIX semaphores and works across
processes. In the patch proposed by Stefan, a *session-wide* mutex is
used. That means that all concurrent git invocations in a user's session
synchronize their initialization of critical section objects.
That's just ridiculous. It's like waiting for a ... no, *the* ... battle
ship just to get our bouncers in their position. We are talking
milliseconds here, not nanoseconds.
-- Hannes
^ permalink raw reply
* Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC
From: Eric Wong @ 2016-10-28 5:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Jeff King, Git Mailing List, Lars Schneider,
Johannes Schindelin
In-Reply-To: <xmqq60od42s0.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> 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.
Actually, I would flip the order of flags. O_CLOEXEC is more
important from a correctness standpoint.
> > 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,
I'm not too familiar with C internals of git; but I know we use
threads in some places, and fork+execve in others.
If our usage of threads and execve intersects, and we run
untrusted code in an execve-ed child, then only having cloexec
on open() will save us time when auditing for leaking FDs.
fcntl(fd, F_SETFD, O_CLOEXEC) is racy in if there are other
threads doing execve; so I wouldn't rely on it as a first
choice.
So I suppose something like this:
static int noatime = 1;
int fd = open(... | O_CLOEXEC);
...error checking and retrying...
if (fd >= 0 && noatime && fcntl(fd, F_SETFL, O_NOATIME) != 0)
noatime = 0;
return fd;
^ 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