* Re: [PATCH v2 10/11] worktree move: refuse to move worktrees with submodules
From: Stefan Beller @ 2016-12-19 20:57 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20161128094319.16176-11-pclouds@gmail.com>
On Mon, Nov 28, 2016 at 1:43 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> +
> + if (S_ISGITLINK(ce->ce_mode)) {
> + found_submodules = 1;
> + break;
> + }
While I applaud being careful with submodules, I think this may be a bit
overeager, because empty (both not initialized as well as not populated)
submodules are fine.
In origin/bw/grep-recurse-submodules^6 you find
int is_submodule_populated(const char *path)
that could be useful here, i.e. I'd imagine you'd change the condition to
if (S_ISGITLINK(ce->ce_mode)
&& !is_submodule_populated(ce->name)) {
...
I guess that can come as a fixup later though.
^ permalink raw reply
* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Jonathan Tan @ 2016-12-19 20:54 UTC (permalink / raw)
To: Kyle J. McKay, Jeff King, Johannes Schindelin
Cc: Junio C Hamano, Git mailing list
In-Reply-To: <A916CED6-C49D-41D8-A7EE-A5FEDA641F4A@gmail.com>
On 12/19/2016 12:38 PM, Kyle J. McKay wrote:
> On Dec 19, 2016, at 12:03, Jeff King wrote:
>
>> On Sat, Dec 17, 2016 at 11:54:18AM -0800, Kyle J. McKay wrote:
>>
>>> Since 6b4b013f18 (mailinfo: handle in-body header continuations,
>>> 2016-09-20, v2.11.0) mailinfo.c has contained new code with an
>>> assert of the form:
>>>
>>> assert(call_a_function(...))
Thanks for spotting this - I'm not sure how I missed that.
>> This is obviously an improvement, but it makes me wonder if we should be
>> doing:
>>
>> if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data))
>> die("BUG: some explanation of why this can never happen");
>>
>> which perhaps documents the intended assumptions more clearly. A comment
>> regarding the side effects might also be helpful.
>
> I wondered exactly the same thing myself. I was hoping Jonathan would
> pipe in here with some analysis about whether this is:
>
> a) a super paranoid, just-in-case, can't really ever fail because by
> the time we get to this code we've already effectively validated
> everything that could cause check_header to return false in this case
>
> -or-
>
> b) Yeah, it could fail in the real world and it should "die" (and
> probably have a test added that triggers such death)
>
> -or-
>
> c) Actually, if check_header does return false we can keep going
> without problem
>
> -or-
>
> d) Actually, if check_header does return false we can keep going by
> making a minor change that should be in the patch
>
> I assume that since Jonathan added the code he will just know the answer
> as to which one it is and I won't have to rely on the results of my
> imaginary analysis. ;)
The answer is "a". The only time that mi->inbody_header_accum is
appended to is in check_inbody_header, and appending onto a blank
mi->inbody_header_accum always happens when is_inbody_header is true
(which guarantees a prefix that causes check_header to always return true).
Peff's suggestion sounds reasonable to me, maybe with an error message
like "BUG: inbody_header_accum, if not empty, must always contain a
valid in-body header".
^ permalink raw reply
* Re: Bug report: $program_name in error message
From: Junio C Hamano @ 2016-12-19 20:50 UTC (permalink / raw)
To: Stefan Beller; +Cc: Josh Bleecher Snyder, vascomalmeida, git@vger.kernel.org
In-Reply-To: <xmqqk2avodi1.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Comparing these changes that involve "\$variable" ...
>
> dashless=$(basename -- "$0" | sed -e 's/-/ /')
> usage() {
> - die "usage: $dashless $USAGE"
> + die "$(eval_gettext "usage: \$dashless \$USAGE")"
> ... and these that appear in the same patch ...
>
> @@ -190,13 +193,16 @@ cd_to_toplevel () {
> require_work_tree_exists () {
> if test "z$(git rev-parse --is-bare-repository)" != zfalse
> then
> - die "fatal: $0 cannot be used without a working tree."
> + program_name=$0
> + die "$(gettext "fatal: \$program_name cannot be used without a working tree.")"
> ...
>
> tells me that the latter needs to be eval_gettext?
Just for fun:
$ git grep -n '[^_]gettext .*\\\$'
shows three hits. Two of them are from that one.
The other is at git-rebase--interactive.sh:440
Perhaps like this to fix.
-- >8 --
Subject: rebase -i: fix mistaken i18n
f2d17068fd ("i18n: rebase-interactive: mark comments of squash for
translation", 2016-06-17) attempted to apply sh-i18n and failed to
use $(eval_gettext "string with \$variable interpolation").
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-rebase--interactive.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 41fd374c72..96865b2375 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -437,7 +437,8 @@ update_squash_messages () {
}' <"$squash_msg".bak
} >"$squash_msg"
else
- commit_message HEAD > "$fixup_msg" || die "$(gettext "Cannot write \$fixup_msg")"
+ commit_message HEAD >"$fixup_msg" ||
+ die "$(eval_gettext "Cannot write \$fixup_msg")"
count=2
{
printf '%s\n' "$comment_char $(gettext "This is a combination of 2 commits.")"
^ permalink raw reply related
* Re: Bug report: $program_name in error message
From: Junio C Hamano @ 2016-12-19 20:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: Josh Bleecher Snyder, vascomalmeida, git@vger.kernel.org
In-Reply-To: <CAGZ79ka=RzAjrb=7u7p5xnveo=kcNCoGn=TC=0j-CBp8Oby7OA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> + Vasco Almeida, who authored d323c6b641,
> (i18n: git-sh-setup.sh: mark strings for translation, 2016-06-17)
Comparing these changes that involve "\$variable" ...
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c48139a494..2eda134800 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -2,6 +2,9 @@
# to set up some variables pointing at the normal git directories and
# a few helper shell functions.
+# Source git-sh-i18n for gettext support.
+. git-sh-i18n
+
# Having this variable in your environment would break scripts because
# you would cause "cd" to be taken to unexpected places. If you
# like CDPATH, define it for your interactive shell sessions without
@@ -83,16 +86,16 @@ if test -n "$OPTIONS_SPEC"; then
else
dashless=$(basename -- "$0" | sed -e 's/-/ /')
usage() {
- die "usage: $dashless $USAGE"
+ die "$(eval_gettext "usage: \$dashless \$USAGE")"
}
if [ -z "$LONG_USAGE" ]
then
- LONG_USAGE="usage: $dashless $USAGE"
+ LONG_USAGE="$(eval_gettext "usage: \$dashless \$USAGE")"
else
- LONG_USAGE="usage: $dashless $USAGE
+ LONG_USAGE="$(eval_gettext "usage: \$dashless \$USAGE
... and these that appear in the same patch ...
@@ -190,13 +193,16 @@ cd_to_toplevel () {
require_work_tree_exists () {
if test "z$(git rev-parse --is-bare-repository)" != zfalse
then
- die "fatal: $0 cannot be used without a working tree."
+ program_name=$0
+ die "$(gettext "fatal: \$program_name cannot be used without a working tree.")"
fi
}
require_work_tree () {
- test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
- die "fatal: $0 cannot be used without a working tree."
+ test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true || {
+ program_name=$0
+ die "$(gettext "fatal: \$program_name cannot be used without a working tree.")"
+ }
}
tells me that the latter needs to be eval_gettext?
^ permalink raw reply related
* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-19 20:38 UTC (permalink / raw)
To: Jeff King, Johannes Schindelin
Cc: Jonathan Tan, Junio C Hamano, Git mailing list
In-Reply-To: <20161219200259.nqqyvk6c72bcoaui@sigill.intra.peff.net>
On Dec 19, 2016, at 12:03, Jeff King wrote:
> On Sat, Dec 17, 2016 at 11:54:18AM -0800, Kyle J. McKay wrote:
>
>> Since 6b4b013f18 (mailinfo: handle in-body header continuations,
>> 2016-09-20, v2.11.0) mailinfo.c has contained new code with an
>> assert of the form:
>>
>> assert(call_a_function(...))
>>
>> The function in question, check_header, has side effects. This
>> means that when NDEBUG is defined during a release build the
>> function call is omitted entirely, the side effects do not
>> take place and tests (fortunately) start failing.
>>
>> Move the function call outside of the assert and assert on
>> the result of the function call instead so that the code
>> still works properly in a release build and passes the tests.
>>
>> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
>> ---
>>
>> Notes:
>> Please include this PATCH in 2.11.x maint
>
> This is obviously an improvement, but it makes me wonder if we
> should be
> doing:
>
> if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data))
> die("BUG: some explanation of why this can never happen");
>
> which perhaps documents the intended assumptions more clearly. A
> comment
> regarding the side effects might also be helpful.
I wondered exactly the same thing myself. I was hoping Jonathan would
pipe in here with some analysis about whether this is:
a) a super paranoid, just-in-case, can't really ever fail because
by the time we get to this code we've already effectively validated
everything that could cause check_header to return false in this case
-or-
b) Yeah, it could fail in the real world and it should "die" (and
probably have a test added that triggers such death)
-or-
c) Actually, if check_header does return false we can keep going
without problem
-or-
d) Actually, if check_header does return false we can keep going by
making a minor change that should be in the patch
I assume that since Jonathan added the code he will just know the
answer as to which one it is and I won't have to rely on the results
of my imaginary analysis. ;)
On Dec 19, 2016, at 09:45, Johannes Schindelin wrote:
> ACK. I noticed this problem (and fixed it independently as a part of a
> huge patch series I did not get around to submit yet) while trying
> to get
> Git to build correctly with Visual C.
Does this mean that Dscho and I are the only ones who add -DNDEBUG for
release builds? Or are we just the only ones who actually run the
test suite on such builds?
--Kyle
^ permalink raw reply
* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Jeff King @ 2016-12-19 20:03 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Jonathan Tan, Junio C Hamano, Git mailing list
In-Reply-To: <900a55073f78a9f19daca67e468d334@3c843fe6ba8f3c586a21345a2783aa0>
On Sat, Dec 17, 2016 at 11:54:18AM -0800, Kyle J. McKay wrote:
> Since 6b4b013f18 (mailinfo: handle in-body header continuations,
> 2016-09-20, v2.11.0) mailinfo.c has contained new code with an
> assert of the form:
>
> assert(call_a_function(...))
>
> The function in question, check_header, has side effects. This
> means that when NDEBUG is defined during a release build the
> function call is omitted entirely, the side effects do not
> take place and tests (fortunately) start failing.
>
> Move the function call outside of the assert and assert on
> the result of the function call instead so that the code
> still works properly in a release build and passes the tests.
>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>
> Notes:
> Please include this PATCH in 2.11.x maint
This is obviously an improvement, but it makes me wonder if we should be
doing:
if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data))
die("BUG: some explanation of why this can never happen");
which perhaps documents the intended assumptions more clearly. A comment
regarding the side effects might also be helpful.
-Peff
^ permalink raw reply
* Re: [PATCH v1] t0021: fix flaky test
From: Jeff King @ 2016-12-19 20:00 UTC (permalink / raw)
To: Ramsay Jones; +Cc: larsxschneider, git, gitster
In-Reply-To: <6e0ea8b0-181a-2ab6-5bab-4f8dfa1d76fa@ramsayjones.plus.com>
On Mon, Dec 19, 2016 at 05:24:32PM +0000, Ramsay Jones wrote:
> > t0021.15 creates files, adds them to the index, and commits them. All
> > this usually happens in a test run within the same second and Git cannot
> > know if the files have been changed between `add` and `commit`. Thus,
> > Git has to run the clean filter in both operations. Sometimes these
> > invocations spread over two different seconds and Git can infer that the
> > files were not changed between `add` and `commit` based on their
> > modification timestamp. The test would fail as it expects the filter
> > invocation. Remove this expectation to make the test stable.
> [...]
> I applied this to the pu branch and ran the test by hand
> 48 times in a row without failure. (the most trials without
> error beforehand was 24).
The original also fails nearly-instantly under my stress script[1], and
it runs for several minutes with this patch.
It might be instructive to try all of the tests under that script, but
it would require a fair bit of patience (and to some degree, people
running "make -j32 test" accomplishes the same thing over time).
-Peff
[1] https://github.com/peff/git/blob/meta/stress
^ permalink raw reply
* Re: [PATCH] winansi_isatty(): fix when Git is used from CMD
From: Johannes Sixt @ 2016-12-19 19:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Pranit Bauva
In-Reply-To: <d661dbf1-9852-965a-2ca9-67d763115b9e@kdbg.org>
Am 18.12.2016 um 16:37 schrieb Johannes Sixt:
> winansi.c is all about overriding MSVCRT's console handling. If we are
> connected to a console, then by the time isatty() is called (from
> outside the emulation layer), all handling of file descriptors 1 and 2
> is already outside MSVCRT's control. In particular, we have determined
> unambiguously whether a terminal is connected (see is_console()). I
> suggest to have the implementation below (on top of the patch I'm
> responding to).
>
> What do you think?
I thought a bit more about this approach, and I retract it. I think it
does not work when Git is connected to an MSYS TTY, i.e., when the
"console" is in reality the pipe that is detected in detect_msys_tty().
At the same time I wonder how your original winansi_isatty() could have
worked: In this case, MSVCRT's isatty() would return 1 (because
detect_msys_tty() has set things up that this happens), but then
winansi_isatty() checks whether the handle underlying fd 0, 1 or 2 is a
real Windows console. But it is not: it is a pipe. Am I missing something?
>
> diff --git a/compat/winansi.c b/compat/winansi.c
> index ba360be69b..1748d17777 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -575,9 +575,8 @@ static void detect_msys_tty(int fd)
>
> int winansi_isatty(int fd)
> {
> - int res = isatty(fd);
> -
> - if (res) {
> + switch (fd) {
> + case 0:
> /*
> * Make sure that /dev/null is not fooling Git into believing
> * that we are connected to a terminal, as "_isatty() returns a
> @@ -586,21 +585,19 @@ int winansi_isatty(int fd)
> *
> * https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
> */
> - HANDLE handle = winansi_get_osfhandle(fd);
> - if (fd == STDIN_FILENO) {
> + {
> + HANDLE handle = (HANDLE)_get_osfhandle(fd);
> DWORD dummy;
>
> - if (!GetConsoleMode(handle, &dummy))
> - res = 0;
> - } else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
> - CONSOLE_SCREEN_BUFFER_INFO dummy;
> -
> - if (!GetConsoleScreenBufferInfo(handle, &dummy))
> - res = 0;
> + return !!GetConsoleMode(handle, &dummy);
> }
> + case 1:
> + return !!hconsole1;
> + case 2:
> + return !!hconsole2;
> }
>
> - return res;
> + return isatty(fd);
> }
>
> void winansi_init(void)
>
^ permalink raw reply
* Re: Suggestion for the "Did you mean this?" feature
From: Kaartic Sivaraam @ 2016-12-19 19:24 UTC (permalink / raw)
To: Chris Packham; +Cc: GIT
In-Reply-To: <CAFOYHZDnpzdYq9j4-xGSdKZQX9deLBpZZhz209qV7cCtq537SA@mail.gmail.com>
Hello all,
On Sun, 18 December 2016 at 20:59, Alexei Lozovsky wrote,
> It's definitely a good thing for human users. For example, I am
> annoyed
> from time to time when I type in some long spell, mistype one minor
> thing,
> and the whole command fails. Then I need to press <up>, correct the
> obvious typo, and run the command again.
>
> Though, there is one aspect which may be the reason why git does not
> have
> this feature: it requires interactive input. For example, it won't
> work
> if some script tries to run an invalid git command. And git cannot
> really
> tell whether it is running interactively or in a batch mode. If it is
> running in batch mode then the whole script may hang indefinitely
> waiting
> for nonexistent input. This also may apply to using git with pipes.
>
> Maybe a configuration option or some GIT_NO_PROMPT environment
> variable
> may be used to force disable this, but it still will be a hassle for
> the
> scripts.
This is a good point that I didn't think of, sir. Thanks for bringing
it up. It seems that in some other form git does have the feature I was
suggesting.
On Mon, 2016-12-19 at 13:48 +1300, Chris Packham wrote:
> This feature already exists (although it's not interactive). See
> help.autoCorrect in the git-config man page. "git config
> help.autoCorrect -1" should to the trick.
Thanks for bringing this to notice, sir. I wasn't aware of it before.
It's in essence the same feature.
On Mon, 2016-12-19 at 12:01 -0500, Marc Branchaud wrote:
> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
> ---
>
> Awesome, I was unaware of this feature. Thanks!
>
> I found the message it prints a bit awkward, so here's a patch to fix
> it up.
>
> Instead of:
>
> WARNING: You called a Git command named 'lgo', which does not
> exist.
> Continuing under the assumption that you meant 'log'
> in 1.5 seconds automatically...
>
> it's now:
>
> WARNING: You called a Git command named 'lgo', which does not
> exist.
> Continuing in 1.5 seconds under the assumption that you meant
> 'log'.
Happy that my mail introduced a little change to git by revealing a not
often used feature.
--
Regards,
Kaartic
^ permalink raw reply
* Re: [PATCH v2 32/34] sequencer (rebase -i): show the progress
From: Junio C Hamano @ 2016-12-19 19:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <9e48dffa8e58183debb79f29413afa81af174475.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> The interactive rebase keeps the user informed about its progress.
> If the sequencer wants to do the grunt work of the interactive
> rebase, it also needs to show that progress.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
Sensible. This further adds two comparisons with TODO_COMMENT and
makes need for is_counted() or something like that felt more.
$ git grep TODO_COMMENT pu sequencer.c
shows that some places "x < TODO_COMMENT" is used while some other
places "y != TODO_COMMENT" is used, both for the same purpose, and
"z >= TODO_COMMENT" is also seen for the negation of the same.
I think all of them except for one can become is_counted() or
!is_counted() to convey what they want to check better while the one
used in the loop control:
for (i = 0; i < TODO_COMMENT; i++)
should probably become:
for (i = 0; i < ARRAY_SIZE(todo_command_info); i++)
as the parsing loop wants to check the input against all known
commands and there is no strong reason for that layer to know how
the insns are numbered. is_xxx() implementation can take advantage
of the way the insns are numbered, but there is no point spreading
the knowledge to higher layer in the callchain.
Other than that, all 34 patches looked sensible steps explained as a
coherent story that unfolds bit by bit, which was mostly a pleasant
read.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 27/34] sequencer (rebase -i): differentiate between comments and 'noop'
From: Junio C Hamano @ 2016-12-19 19:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <b82347c627fd3b8a74827bc773a5df2d16c6dded.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> In the upcoming patch, we will support rebase -i's progress
> reporting. The progress skips comments but counts 'noop's.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
> diff --git a/sequencer.c b/sequencer.c
> index 1f314b2743..63f6f25ced 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -770,7 +770,9 @@ enum todo_command {
> TODO_EXEC,
> /* commands that do nothing but are counted for reporting progress */
> TODO_NOOP,
> - TODO_DROP
> + TODO_DROP,
> + /* comments (not counted for reporting progress) */
> + TODO_COMMENT
> };
>
> static struct {
Makes sense. I would have done this immediately after introducing
NOOP if I were doing this series, if only because by having the
unchanging last element early in enum {} definition, we can avoid
having to deal with the "last element cannot have comma", but that
is not a big issue.
> @@ -785,12 +787,13 @@ static struct {
> { 's', "squash" },
> { 'x', "exec" },
> { 0, "noop" },
> - { 'd', "drop" }
> + { 'd', "drop" },
> + { 0, NULL }
> };
>
> static const char *command_to_string(const enum todo_command command)
> {
> - if ((size_t)command < ARRAY_SIZE(todo_command_info))
> + if (command < TODO_COMMENT)
> return todo_command_info[command].str;
> die("Unknown command: %d", command);
> }
The same comment as "instead of comparing with TODO_NOOP, you would
want is_noop()" applies to three instances of comparing with
TODO_COMMENT we can see in this patch, I think.
"is_counted()" perhaps?
^ permalink raw reply
* Re: [PATCH v2 24/34] sequencer (rebase -i): respect strategy/strategy_opts settings
From: Junio C Hamano @ 2016-12-19 18:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <a21233f368f066408051e6bdc9a2b6ec513e9e11.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> The sequencer already has an idea about using different merge
> strategies. We just piggy-back on top of that, using rebase -i's
> own settings, when running the sequencer in interactive rebase mode.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
A handful of steps before and including this one look quite faithful
port to C from the scripted one.
Looking good.
^ permalink raw reply
* Re: [PATCH 3/3] push: add option to push only submodules
From: Stefan Beller @ 2016-12-19 18:56 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org
In-Reply-To: <1482171933-180601-4-git-send-email-bmwill@google.com>
On Mon, Dec 19, 2016 at 10:25 AM, Brandon Williams <bmwill@google.com> wrote:
> Teach push the --recurse-submodules=only option. This enables push to
> recursively push all unpushed submodules while leaving the superproject
> unpushed.
>
> This is a desirable feature in a scenario where updates to the
> superproject are handled automatically by some other means, perhaps a
e.g. Gerrit. (No need to be shy about our code review tool)
>
> code review tool. In this scenario a developer could make a change
> which spans multiple submodules and then push their commits for code
> review. Upon completion of the code review, their commits can be
> accepted and applied to their respective submodules while the code
> review tool can then automatically update the superproject to the most
> recent SHA1 of each submodule. This would eliminate the merge conflicts
> in the superproject that could occur if multiple people are contributing
> to the same submodule.
Code and tests look good to me, I think the commit message
is good enough, but let's hear Junio on this one.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v2 03/34] sequencer (rebase -i): implement the 'edit' command
From: Junio C Hamano @ 2016-12-19 18:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1612191438200.54750@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > + strbuf_addf(&buf, "%s/patch", get_dir(opts));
>> > + memset(&log_tree_opt, 0, sizeof(log_tree_opt));
>> > + init_revisions(&log_tree_opt, NULL);
>> > + log_tree_opt.abbrev = 0;
>> > + log_tree_opt.diff = 1;
>> > + log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
>> > + log_tree_opt.disable_stdin = 1;
>> > + log_tree_opt.no_commit_id = 1;
>> > + log_tree_opt.diffopt.file = fopen(buf.buf, "w");
>> > + log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
>> > + if (!log_tree_opt.diffopt.file)
>> > + res |= error_errno(_("could not open '%s'"), buf.buf);
>> > + else {
>> > + res |= log_tree_commit(&log_tree_opt, commit);
>> > + fclose(log_tree_opt.diffopt.file);
>> > + }
>> > + strbuf_reset(&buf);
>> > + strbuf_addf(&buf, "%s/message", get_dir(opts));
>> > + if (!file_exists(buf.buf)) {
>> > + find_commit_subject(commit_buffer, &subject);
>> > + res |= write_message(subject, strlen(subject), buf.buf, 1);
>> > + unuse_commit_buffer(commit, commit_buffer);
>> > + }
>> > + strbuf_release(&buf);
>> > +
>> > + return res;
>> > +}
>>
>> OK. This seems to match what scripted make_patch does in a handful
>> of lines. We probably should have given you a helper to reduce
>> boilerplate that sets up log_tree_opt so that this function does not
>> have to be this long, but that is a separate topic.
>>
>> Does it matter output_format is set to FORMAT_PATCH here, though?
>> With --no-commit-id set, I suspect there is no log message or
>> authorship information given to the output.
Sorry, this was me being stupid.
FORMAT_PATCH here does not have anythning to do with "git
format-patch" (and "git log --pretty=email"). The PATCH there is as
opposed to things like --stat and --raw. We want patch text that
can be fed to "git apply" and it is absolutely the right thing to
use here.
^ permalink raw reply
* Re: [PATCH v2 03/34] sequencer (rebase -i): implement the 'edit' command
From: Junio C Hamano @ 2016-12-19 18:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1612191438200.54750@virtualbox>
^ permalink raw reply
* Re: [PATCH 0/3] push only submodules
From: Junio C Hamano @ 2016-12-19 18:41 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller
In-Reply-To: <1482171933-180601-1-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> This series teaches 'git push' to be able to only push submodules
> while leaving a superproject unpushed.
> ...
> builtin/push.c | 2 ++
My knee-jerk reaction is "why is this even part of 'git push' if it
does not push?"
I think "git submodule foreach git push" is probably a mouthful to
say, but I am not sure "git push --recurse-submodule=only" is a
short-hand that is way better than that.
Maybe I'll find why the latter is better after reading the patches
through ;-)
^ permalink raw reply
* Re: [PATCH v2 02/34] sequencer (rebase -i): implement the 'noop' command
From: Junio C Hamano @ 2016-12-19 18:31 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Linus Torvalds, Git Mailing List, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1612191237400.54750@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Tue, 13 Dec 2016, Junio C Hamano wrote:
>
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>>
>> > On Tue, Dec 13, 2016 at 12:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> >>
>> >>> +/*
>> >>> + * Note that ordering matters in this enum. Not only must it match the mapping
>> >>> + * below, it is also divided into several sections that matter. When adding
>> >>> + * new commands, make sure you add it in the right section.
>> >>> + */
>> >>
>> >> Good thinking.
>
> Not my thinking... This was in direct response to a suggestion by Dennis
> Kaarsemaker, I cannot take credit for the idea.
I now realize that I was unclear about what "thinking" I found good
in my comment. I do not particularly like defining two parallel
things and having to maintain them in sync. The "Good thinking"
praise goes to whoever thought that this burdensome fact deserves a
clear comment in front of these two things.
And ...
>> Makes me wish C were a better language, though ;-)
>> >
>> > Do this:
>> >
>> > static const char *todo_command_strings[] = {
>> > [TODO_PICK] = "pick",
>> > [TODO_REVERT] = "revert",
>> > [TODO_NOOP] = "noop:,
>> > };
>> >
>> > which makes the array be order-independent.
... solves only one-half of the problem with the language I had.
The order of the entries in this array[] may become more flexible
in the source, but you still have to define enum separately.
I guess if we really want to, we need to resort to something "ugly
but workable" like what you did in fsck.c with FOREACH_MSG_ID(X).
That approach may be the least ugly way if we have to maintain two
or more parallel things in sync.
... and then realizes you wrote pretty much the same thing
... after writing all of the above ;-)
But it is way overkill for sequencer commands that are only handful.
^ permalink raw reply
* [PATCH 3/3] push: add option to push only submodules
From: Brandon Williams @ 2016-12-19 18:25 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <1482171933-180601-1-git-send-email-bmwill@google.com>
Teach push the --recurse-submodules=only option. This enables push to
recursively push all unpushed submodules while leaving the superproject
unpushed.
This is a desirable feature in a scenario where updates to the
superproject are handled automatically by some other means, perhaps a
code review tool. In this scenario a developer could make a change
which spans multiple submodules and then push their commits for code
review. Upon completion of the code review, their commits can be
accepted and applied to their respective submodules while the code
review tool can then automatically update the superproject to the most
recent SHA1 of each submodule. This would eliminate the merge conflicts
in the superproject that could occur if multiple people are contributing
to the same submodule.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/push.c | 2 ++
t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++
transport.c | 15 +++++++++++----
transport.h | 1 +
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..9433797 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -565,6 +565,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
flags |= TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND;
+ else if (recurse_submodules == RECURSE_SUBMODULES_ONLY)
+ flags |= TRANSPORT_RECURSE_SUBMODULES_ONLY;
if (tags)
add_refspec("refs/tags/*");
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 1524ff5..676d686 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -454,4 +454,25 @@ test_expect_success 'push --dry-run does not recursively update submodules' '
test_cmp expected_submodule actual_submodule
'
+test_expect_success 'push --dry-run does not recursively update submodules' '
+ git -C work push --dry-run --recurse-submodules=only ../pub.git master &&
+
+ git -C submodule.git rev-parse master >actual_submodule &&
+ git -C pub.git rev-parse master >actual_pub &&
+ test_cmp expected_pub actual_pub &&
+ test_cmp expected_submodule actual_submodule
+'
+
+test_expect_success 'push only unpushed submodules recursively' '
+ git -C pub.git rev-parse master >expected_pub &&
+ git -C work/gar/bage rev-parse master >expected_submodule &&
+
+ git -C work push --recurse-submodules=only ../pub.git master &&
+
+ git -C submodule.git rev-parse master >actual_submodule &&
+ git -C pub.git rev-parse master >actual_pub &&
+ test_cmp expected_submodule actual_submodule &&
+ test_cmp expected_pub actual_pub
+'
+
test_done
diff --git a/transport.c b/transport.c
index 04e5d66..20ebee8 100644
--- a/transport.c
+++ b/transport.c
@@ -947,7 +947,9 @@ int transport_push(struct transport *transport,
if (run_pre_push_hook(transport, remote_refs))
return -1;
- if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
+ if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
+ TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&
+ !is_bare_repository()) {
struct ref *ref = remote_refs;
struct sha1_array commits = SHA1_ARRAY_INIT;
@@ -965,7 +967,8 @@ int transport_push(struct transport *transport,
}
if (((flags & TRANSPORT_RECURSE_SUBMODULES_CHECK) ||
- ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) &&
+ ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
+ TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&
!pretend)) && !is_bare_repository()) {
struct ref *ref = remote_refs;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
@@ -984,7 +987,10 @@ int transport_push(struct transport *transport,
sha1_array_clear(&commits);
}
- push_ret = transport->push_refs(transport, remote_refs, flags);
+ if (!(flags & TRANSPORT_RECURSE_SUBMODULES_ONLY))
+ push_ret = transport->push_refs(transport, remote_refs, flags);
+ else
+ push_ret = 0;
err = push_had_errors(remote_refs);
ret = push_ret | err;
@@ -996,7 +1002,8 @@ int transport_push(struct transport *transport,
if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
set_upstreams(transport, remote_refs, pretend);
- if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
+ if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
+ TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
struct ref *ref;
for (ref = remote_refs; ref; ref = ref->next)
transport_update_tracking_ref(transport->remote, ref, verbose);
diff --git a/transport.h b/transport.h
index 1b65458..efd5fb6 100644
--- a/transport.h
+++ b/transport.h
@@ -146,6 +146,7 @@ struct transport {
#define TRANSPORT_PUSH_CERT_IF_ASKED (1<<12)
#define TRANSPORT_PUSH_ATOMIC (1<<13)
#define TRANSPORT_PUSH_OPTIONS (1<<14)
+#define TRANSPORT_RECURSE_SUBMODULES_ONLY (1<<15)
extern int transport_summary_width(const struct ref *refs);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 2/3] submodules: add RECURSE_SUBMODULES_ONLY value
From: Brandon Williams @ 2016-12-19 18:25 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <1482171933-180601-1-git-send-email-bmwill@google.com>
Add the `RECURSE_SUBMODULES_ONLY` enum value to submodule.h. This enum
value will be used in a later patch to push to indicate that only
submodules should be pushed, while the superproject should remain
unpushed.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule-config.c | 2 ++
submodule.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/submodule-config.c b/submodule-config.c
index 098085b..33eb62d 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -251,6 +251,8 @@ static int parse_push_recurse(const char *opt, const char *arg,
return RECURSE_SUBMODULES_ON_DEMAND;
else if (!strcmp(arg, "check"))
return RECURSE_SUBMODULES_CHECK;
+ else if (!strcmp(arg, "only"))
+ return RECURSE_SUBMODULES_ONLY;
else if (die_on_error)
die("bad %s argument: %s", opt, arg);
else
diff --git a/submodule.h b/submodule.h
index 23d7668..4a83a4c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -6,6 +6,7 @@ struct argv_array;
struct sha1_array;
enum {
+ RECURSE_SUBMODULES_ONLY = -5,
RECURSE_SUBMODULES_CHECK = -4,
RECURSE_SUBMODULES_ERROR = -3,
RECURSE_SUBMODULES_NONE = -2,
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 1/3] transport: refactor flag #defines to be more readable
From: Brandon Williams @ 2016-12-19 18:25 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
In-Reply-To: <1482171933-180601-1-git-send-email-bmwill@google.com>
All of the #defines for the TRANSPORT_* flags are hardcoded to be powers
of two. This can be error prone when adding a new flag and is difficult
to read. This patch refactors these defines to instead use a shift
operation to generate the flags. This makes adding an additional flag
simpilar and makes the defines easier to read.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
transport.h | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/transport.h b/transport.h
index b8e4ee8..1b65458 100644
--- a/transport.h
+++ b/transport.h
@@ -131,21 +131,21 @@ struct transport {
enum transport_family family;
};
-#define TRANSPORT_PUSH_ALL 1
-#define TRANSPORT_PUSH_FORCE 2
-#define TRANSPORT_PUSH_DRY_RUN 4
-#define TRANSPORT_PUSH_MIRROR 8
-#define TRANSPORT_PUSH_PORCELAIN 16
-#define TRANSPORT_PUSH_SET_UPSTREAM 32
-#define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
-#define TRANSPORT_PUSH_PRUNE 128
-#define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256
-#define TRANSPORT_PUSH_NO_HOOK 512
-#define TRANSPORT_PUSH_FOLLOW_TAGS 1024
-#define TRANSPORT_PUSH_CERT_ALWAYS 2048
-#define TRANSPORT_PUSH_CERT_IF_ASKED 4096
-#define TRANSPORT_PUSH_ATOMIC 8192
-#define TRANSPORT_PUSH_OPTIONS 16384
+#define TRANSPORT_PUSH_ALL (1<<0)
+#define TRANSPORT_PUSH_FORCE (1<<1)
+#define TRANSPORT_PUSH_DRY_RUN (1<<2)
+#define TRANSPORT_PUSH_MIRROR (1<<3)
+#define TRANSPORT_PUSH_PORCELAIN (1<<4)
+#define TRANSPORT_PUSH_SET_UPSTREAM (1<<5)
+#define TRANSPORT_RECURSE_SUBMODULES_CHECK (1<<6)
+#define TRANSPORT_PUSH_PRUNE (1<<7)
+#define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND (1<<8)
+#define TRANSPORT_PUSH_NO_HOOK (1<<9)
+#define TRANSPORT_PUSH_FOLLOW_TAGS (1<<10)
+#define TRANSPORT_PUSH_CERT_ALWAYS (1<<11)
+#define TRANSPORT_PUSH_CERT_IF_ASKED (1<<12)
+#define TRANSPORT_PUSH_ATOMIC (1<<13)
+#define TRANSPORT_PUSH_OPTIONS (1<<14)
extern int transport_summary_width(const struct ref *refs);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 0/3] push only submodules
From: Brandon Williams @ 2016-12-19 18:25 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
This series teaches 'git push' to be able to only push submodules while leaving
a superproject unpushed.
This is a desirable feature in a scenario where updates to the
superproject are handled automatically by some other means, perhaps a
code review tool. In this scenario a developer could make a change
which spans multiple submodules and then push their commits for code
review. Upon completion of the code review, their commits can be
accepted and applied to their respective submodules while the code
review tool can then automatically update the superproject to the most
recent SHA1 of each submodule. This would eliminate the merge conflicts
in the superproject that could occur if multiple people are contributing
to the same submodule.
Brandon Williams (3):
transport: refactor flag #defines to be more readable
submodules: add RECURSE_SUBMODULES_ONLY value
push: add option to push only submodules
builtin/push.c | 2 ++
submodule-config.c | 2 ++
submodule.h | 1 +
t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++
transport.c | 15 +++++++++++----
transport.h | 31 ++++++++++++++++---------------
6 files changed, 53 insertions(+), 19 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: [PATCH 0/5] git check-ref-format --stdin --report-errors
From: Junio C Hamano @ 2016-12-19 18:23 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Ian Jackson, git
In-Reply-To: <561c0338-66cd-f806-7b3b-b422f98a1564@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Especially given that the output is not especially machine-readable, it
> might be more consistent with other commands to call the new feature
> `--verbose` rather than `--report-errors`.
Don't we instead want to structure the output to be machine-readable
instead, given that check-ref-format is a very low level plumbing
command that is primarily meant for scriptors?
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #02; Mon, 12)
From: Junio C Hamano @ 2016-12-19 18:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1612191145080.54750@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Otherwise I'd prefer to drop it---at this point, the series is merely
>> "just because we can", not "because we need it to further improve this
>> or that".
>
> Oh, I thought that this was meant as a starting point for anybody
> interested in playing with resumable clones or with easing server loads.
It started as such, but the effort stalled and nobody seems to be
taking it further. I do not know it is clear it would be a good
starting point at this point. Perhaps it is.
^ permalink raw reply
* Re: [PATCHv8 6/6] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-19 18:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git@vger.kernel.org, Brandon Williams
In-Reply-To: <20161219053507.GA2335@duynguyen.vn.dektech.internal>
On Sun, Dec 18, 2016 at 9:35 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 11:04:35AM -0800, Stefan Beller wrote:
>> diff --git a/dir.c b/dir.c
>> index e0efd3c2c3..d872cc1570 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -2773,3 +2773,15 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>> free(work_tree);
>> free(git_dir);
>> }
>> +
>> +/*
>> + * Migrate the git directory of the given path from old_git_dir to new_git_dir.
>> + */
>> +void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
>> +{
>> + if (rename(old_git_dir, new_git_dir) < 0)
>> + die_errno(_("could not migrate git directory from '%s' to '%s'"),
>> + old_git_dir, new_git_dir);
>> +
>> + connect_work_tree_and_git_dir(path, new_git_dir);
>
> Should we worry about recovering (e.g. maybe move new_git_dir back to
> old_git_dir) if this connect_work_tree_and_git_dir() fails?
What if the move back fails?
>
> Both write_file() and git_config_set_.. in this function may die(). In
> such a case the repo is in broken state and the user needs pretty good
> submodule understanding to recover from it, I think.
>
> Recovering is not easy (nor entirely safe) either, though I suppose if
> we keep original copies for modified files, then we could restore them
> after moving the directory back and pray the UNIX gods that all
> operations succeed.
There are different levels of brokenness available.
I just tried what happens if core.worktree is unset in a submodule
and that seems to not impact git operations (I only tested git-status
both in the superproject as well as in the submodule).
So I wonder why we set core.worktree at all here as it doesn't
seem to be needed.
Which means that the move of the git directory as well as the .git file
update to point at that moved directory are the important things
to get right.
So maybe:
1) rename the git dir or die
2) write the new gitlink
If that fails remove the .git file (if it exists partially or empty)
and undo 1) by calling rename again with swapped arguments
and then die
3) set core.worktree
If that fails, just warn the user
^ permalink raw reply
* Re: [PATCH] config.c: handle error case for fstat() calls
From: Junio C Hamano @ 2016-12-19 18:14 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, josharian
In-Reply-To: <20161219092155.20359-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Will this fix the problem I'm replying to? I don't know. I found this
> while checking the code and it should be fixed regardless.
Yeah, from a cursory read, it is a step in the right direction to
check the return value of fstat().
Shouldn't the error-return path in the second hunk rollback the
lockfile to clean after itself? The existing "Oh, we cannot chmod
to match the original" check that comes immediately after shares the
same issue, so this is not a new problem, but making an existing one
worse.
> config.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index 83fdecb..4973256 100644
> --- a/config.c
> +++ b/config.c
> @@ -2194,7 +2194,12 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
> goto out_free;
> }
>
> - fstat(in_fd, &st);
> + if (fstat(in_fd, &st) == -1) {
> + error_errno(_("fstat on %s failed"), config_filename);
> + ret = CONFIG_INVALID_FILE;
> + goto out_free;
> + }
> +
> contents_sz = xsize_t(st.st_size);
> contents = xmmap_gently(NULL, contents_sz, PROT_READ,
> MAP_PRIVATE, in_fd, 0);
> @@ -2414,7 +2419,10 @@ int git_config_rename_section_in_file(const char *config_filename,
> goto unlock_and_out;
> }
>
> - fstat(fileno(config_file), &st);
> + if (fstat(fileno(config_file), &st) == -1) {
> + ret = error_errno(_("fstat on %s failed"), config_filename);
> + goto out;
> + }
>
> if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
> ret = error_errno("chmod on %s failed",
^ 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