* Re: [PATCH] fast-import: properly fanout notes when tree is imported
From: Johan Herland @ 2016-12-19 22:05 UTC (permalink / raw)
To: Mike Hommey; +Cc: Git mailing list, Junio C Hamano
In-Reply-To: <20161219021212.15978-1-mh@glandium.org>
On Mon, Dec 19, 2016 at 3:12 AM, Mike Hommey <mh@glandium.org> wrote:
> In typical uses of fast-import, trees are inherited from a parent
> commit. In that case, the tree_entry for the branch looks like:
>
> .versions[1].sha1 = $some_sha1
> .tree = <tree structure loaded from $some_sha1>
>
> However, when trees are imported, rather than inherited, that is not the
> case. One can import a tree with a filemodify command, replacing the
> root tree object.
>
> e.g.
> "M 040000 $some_sha1 \n"
>
> In this case, the tree_entry for the branch looks like:
>
> .versions[1].sha1 = $some_sha1
> .tree = NULL
>
> When adding new notes with the notemodify command, do_change_note_fanout
> is called to get a notes count, and to do so, it loops over the
> tree_entry->tree, but doesn't do anything when the tree is NULL.
>
> In the latter case above, it means do_change_note_fanout thinks the tree
> contains no note, and new notes are added with no fanout.
s/note,/notes,/
>
> Interestingly, do_change_note_fanout does check whether subdirectories
> have a NULL .tree, in which case it uses load_tree(). Which means the
> right behaviour happens when using the filemodify command to import
> subdirectories.
>
> This change makes do_change_note_fanount call load_tree() whenever the
> tree_entry it is given has no tree loaded, making all cases handled
> equally.
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
Acked-by: Johan Herland <johan@herland.net>
^ permalink raw reply
* Re: [PATCH 00/13] gitk: tweak rendering of remote-tracking references
From: Philip Oakley @ 2016-12-19 22:53 UTC (permalink / raw)
To: Michael Haggerty, Paul Mackerras; +Cc: Git List, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
From: "Michael Haggerty" <mhagger@alum.mit.edu>
> This patch series changes a bunch of details about how remote-tracking
> references are rendered in the commit list of gitk:
>
[...]
> * Introduce a separate constant to specify the background color used
> for the branch name part of remote-tracking references, to allow it
> to differ from the color used for local branches (which by default
> is bright green).
>
> * Change the default background colors for remote-tracking branches to
> light brown and brown (formerly they were pale orange and bright
> green).
>
> I understand that the colors of pixels on computer screens is an even
> more emotional topic that that of bikesheds, so I implemented the last
> change as a separate commit, the last one in the series. Feel free to
> drop it if you don't want the default color change.
>
Just to say that there was an issue with the bright green (lime) a while
back when 'green' changed its colour.
dscho reported in
(https://github.com/git-for-windows/git/issues/300#issuecomment-133702654
26 Aug 2015)
"[T]his is a change in Tk 8.6 described here (http://wiki.tcl.tk/1424): From
Tcl/Tk 8.6 on, Tk uses Web colours instead of X11 ones, where they
conflict."
In particular the old bright green version of 'green' became a darker green,
with the old colour becoming named lime.
For me, I needed to change my colour scheme (to a lime) as I could not read
the refs against the darker colour.
Anyway, that's the background as I know it.
--
Philip
^ permalink raw reply
* Re: [PATCH 0/2] improve relocate_git_dir to fail into a sane state.
From: Junio C Hamano @ 2016-12-19 22:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: pclouds, bmwill, git
In-Reply-To: <20161219215709.24620-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This goes on top of sb/submodule-embed-gitdir.
> When the absorbing of a git dir fails, try to recover into a sane state,
> i.e. try to undo the move of the git dir.
Are these unconditionally good improvements? I ask because the
series is still not in 'next' and it is the last chance to fold
these into existing patches if we wanted to.
>
> Thanks,
> Stefan
>
> Stefan Beller (2):
> dir.c: split up connect_work_tree_and_git_dir
> dir.c: add retry logic to relocate_gitdir
>
> dir.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> dir.h | 6 ++--
> submodule.c | 3 +-
> 3 files changed, 110 insertions(+), 17 deletions(-)
^ permalink raw reply
* [PATCH v2] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-19 23:13 UTC (permalink / raw)
To: Junio C Hamano, Jeff King, Johannes Schindelin, Jonathan Tan
Cc: Git mailing list
In-Reply-To: <xmqqbmw7ocoz.fsf@gitster.mtv.corp.google.com>
On Dec 19, 2016, at 13:01, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>>>> 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
>>> ...
>> 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".
>
> OK. So we do not expect it to fail, but we still do want the side
> effect of that function (i.e. accmulation into the field).
>
> Somebody care to send a final "agreed-upon" version?
Yup, here it is:
-- 8< --
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.
Since 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, this guarantees a prefix that causes check_header to always
return true.
Therefore replace the assert with an if !check_header + DIE
combination to reflect this.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
Notes:
Please include this PATCH in 2.11.x maint
mailinfo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mailinfo.c b/mailinfo.c
index 2fb3877e..a489d9d0 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -710,7 +710,8 @@ static void flush_inbody_header_accum(struct mailinfo *mi)
{
if (!mi->inbody_header_accum.len)
return;
- assert(check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0));
+ if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0))
+ die("BUG: inbody_header_accum, if not empty, must always contain a valid in-body header");
strbuf_reset(&mi->inbody_header_accum);
}
---
^ permalink raw reply related
* Re: [PATCH 0/2] improve relocate_git_dir to fail into a sane state.
From: Stefan Beller @ 2016-12-19 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Brandon Williams, git@vger.kernel.org
In-Reply-To: <xmqqfuljmswl.fsf@gitster.mtv.corp.google.com>
On Mon, Dec 19, 2016 at 2:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This goes on top of sb/submodule-embed-gitdir.
>> When the absorbing of a git dir fails, try to recover into a sane state,
>> i.e. try to undo the move of the git dir.
>
> Are these unconditionally good improvements? I ask because the
> series is still not in 'next' and it is the last chance to fold
> these into existing patches if we wanted to.
>
Actually I forgot to mark these as RFC-ish as it is a response to
https://public-inbox.org/git/20161219053507.GA2335@duynguyen.vn.dektech.internal/
(which was the only review comment this round)
So I'd say these patches are rather experimental in my mind
unlike the absorb series, which I assume is ok as is already.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 2/2] dir.c: add retry logic to relocate_gitdir
From: Stefan Beller @ 2016-12-19 23:19 UTC (permalink / raw)
To: Brandon Williams; +Cc: Duy Nguyen, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20161219222551.GA41080@google.com>
On Mon, Dec 19, 2016 at 2:25 PM, Brandon Williams <bmwill@google.com> wrote:
> On 12/19, Stefan Beller wrote:
>> Relocating a git directory consists of 3 steps:
>> 1) Move the directory.
>> 2) Update the gitlink file.
>> 3) Set core.worktree correctly.
>>
>> In an ideal world all three steps would be part of one transaction, such
>> that either all of them happen correctly or none of them.
>> However currently we just execute these three steps sequentially and die
>> in case of an error in any of these 3 steps.
>>
>> Dying is ok in 1) as the transaction hasn't started and the state is
>> recoverable.
>>
>> When dying in 2), this is a problem as the repo state is left in an
>> inconsistent state, e.g. the git link file of a submodule could be
>> empty and hence even the superproject appears to be broken as basic
>> commands such as git-status would die as there is it cannot tell the
>> state of the submodule.
>> So in that case try to undo 1) to be in a less sufferable state.
>
> I now see why an atomic filesystem swap operation would be useful :)
>
>>
>> 3) is less of an issue as experiments with submodules show. When
>> core.worktree is unset or set incorrectly, git-status still works
>> both in the superproject as well as the working tree of the submodule.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> dir.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>> dir.h | 6 ++--
>> submodule.c | 3 +-
>> 3 files changed, 91 insertions(+), 12 deletions(-)
>>
>> diff --git a/dir.c b/dir.c
>> index b2cb23fe88..e4e3f69869 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -2749,30 +2749,66 @@ void untracked_cache_add_to_index(struct index_state *istate,
>> untracked_cache_invalidate_path(istate, path);
>> }
>>
>> -static void point_gitlink_file_to(const char *work_tree, const char *git_dir)
>> +/*
>> + * Just like write_file, we try hard to write the full content to the file.
>> + * If there is suspicion the write did not work correctly, make sure the file
>> + * is removed again.
>> + * Return 0 if the write succeeded, -1 if the file was removed,
>> + * -2 if the (partial) file is still there.
>> + */
>> +static int write_file_or_remove(const char *path, const char *buf, size_t len)
>> +{
>> + int retries = 3;
>> + int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
>> + if (write_in_full(fd, buf, len) != len) {
>> + warning_errno(_("could not write '%s'"), path);
>> + goto err;
>> + }
>> + if (close(fd)) {
>> + warning_errno(_("could not close '%s'"), path);
>> + goto err;
>> + }
>> + return 0;
>> +err:
>> + while (retries-- > 0) {
>> + if (file_exists(path))
>> + unlink_or_warn(path);
>> + else
>> + return -1;
>> + }
>> + return -2;
>> +}
>
> Any reason why on attempt 2 or 3 this would succeed if it failed on try
> 1?
>
This code is heavily inspired by refs/files-backend.c which upon
closer inspection only retries directory things within the git directory
(which is assumed to be accessed in parallel by different invocations
of Git)
So I think we could drop the retry logic.
>> {
>> + char *git_dir = xstrdup(real_path(new_git_dir));
>> + char *work_tree = xstrdup(real_path(path));
>> + int c;
>
> I guess in a later patch we could clean up these calls to real_path to
> use real_pathdup from bw/realpath-wo-chdir.
good point.
^ permalink raw reply
* Re: [PATCH v2] mailinfo.c: move side-effects outside of assert
From: Junio C Hamano @ 2016-12-19 23:26 UTC (permalink / raw)
To: Kyle J. McKay
Cc: Jeff King, Johannes Schindelin, Jonathan Tan, Git mailing list
In-Reply-To: <ebaf4c892a78bc3ae614a23d87f9c0f@58437222ff6db9ee7cbe9d1a5a1ad4e>
"Kyle J. McKay" <mackyle@gmail.com> writes:
>> OK. So we do not expect it to fail, but we still do want the side
>> effect of that function (i.e. accmulation into the field).
>>
>> Somebody care to send a final "agreed-upon" version?
>
> Yup, here it is:
Thanks.
> -- 8< --
>
> 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.
>
> Since 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, this guarantees a prefix that causes check_header to always
> return true.
>
> Therefore replace the assert with an if !check_header + DIE
> combination to reflect this.
>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Jeff King <peff@peff.net>
> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>
> Notes:
> Please include this PATCH in 2.11.x maint
>
> mailinfo.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mailinfo.c b/mailinfo.c
> index 2fb3877e..a489d9d0 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -710,7 +710,8 @@ static void flush_inbody_header_accum(struct mailinfo *mi)
> {
> if (!mi->inbody_header_accum.len)
> return;
> - assert(check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0));
> + if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0))
> + die("BUG: inbody_header_accum, if not empty, must always contain a valid in-body header");
> strbuf_reset(&mi->inbody_header_accum);
> }
>
> ---
^ permalink raw reply
* Re: [PATCH 0/2] improve relocate_git_dir to fail into a sane state.
From: Junio C Hamano @ 2016-12-19 23:27 UTC (permalink / raw)
To: Stefan Beller; +Cc: Duy Nguyen, Brandon Williams, git@vger.kernel.org
In-Reply-To: <CAGZ79kZA1HxsdCAQxKiNMQrH9fLrhGKc8Um7x-R7u92ujVBigQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Dec 19, 2016 at 2:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> This goes on top of sb/submodule-embed-gitdir.
>>> When the absorbing of a git dir fails, try to recover into a sane state,
>>> i.e. try to undo the move of the git dir.
>>
>> Are these unconditionally good improvements? I ask because the
>> series is still not in 'next' and it is the last chance to fold
>> these into existing patches if we wanted to.
>
> Actually I forgot to mark these as RFC-ish as it is a response to
> https://public-inbox.org/git/20161219053507.GA2335@duynguyen.vn.dektech.internal/
> (which was the only review comment this round)
>
> So I'd say these patches are rather experimental in my mind
> unlike the absorb series, which I assume is ok as is already.
OK. Thanks for clarification.
^ permalink raw reply
* [PATCHv4 0/5] git-rm absorbs submodule git directory before deletion
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
v4:
* reworded commit messages of the last 2 patches
* introduced a new patch introducing {run,start,finish}_command_or_die
* found an existing function in dir.h to use to remove a directory
which deals gracefully with the cornercases, that Junio pointed out.
v3:
* removed the patch to enhance ok_to_remove_submodule to absorb the submodule
if needed
* Removed all the error reporting from git-rm that was related to submodule
git directories not absorbed.
* instead just absorb the git repositories or let the absorb function die
with an appropriate error message.
v2:
* new base where to apply the patch:
sb/submodule-embed-gitdir merged with sb/t3600-cleanup.
I got merge conflicts and resolved them this way:
#@@@ -709,9 -687,10 +687,9 @@@ test_expect_success 'checking out a com
# git commit -m "submodule removal" submod &&
# git checkout HEAD^ &&
# git submodule update &&
#- git checkout -q HEAD^ 2>actual &&
#+ git checkout -q HEAD^ &&
# git checkout -q master 2>actual &&
# - echo "warning: unable to rmdir submod: Directory not empty" >expected &&
# - test_i18ncmp expected actual &&
# + test_i18ngrep "^warning: unable to rmdir submod:" actual &&
# git status -s submod >actual &&
# echo "?? submod/" >expected &&
# test_cmp expected actual &&
#
* improved commit message in "ok_to_remove_submodule: absorb the submodule git dir"
(David Turner offered me some advice on how to write better English off list)
* simplified code in last patch:
-> dropped wrong comment for fallthrough
-> moved redundant code out of both bodies of an if-clause.
* Fixed last patchs commit message to have "or_die" instead of or_dir.
v1:
The "checkout --recurse-submodules" series got too large to comfortably send
it out for review, so I had to break it up into smaller series'; this is the
first subseries, but it makes sense on its own.
This series teaches git-rm to absorb the git directory of a submodule instead
of failing and complaining about the git directory preventing deletion.
It applies on origin/sb/submodule-embed-gitdir.
Any feedback welcome!
Thanks,
Stefan
Stefan Beller (5):
submodule.h: add extern keyword to functions
submodule: modernize ok_to_remove_submodule to use argv_array
run-command: add {run,start,finish}_command_or_die
submodule: add flags to ok_to_remove_submodule
rm: absorb a submodules git dir before deletion
builtin/rm.c | 82 +++++++++++++++--------------------------------------------
run-command.c | 28 ++++++++++++++++++++
run-command.h | 4 +++
submodule.c | 27 ++++++++++----------
submodule.h | 58 ++++++++++++++++++++++++------------------
t/t3600-rm.sh | 39 +++++++++++-----------------
6 files changed, 114 insertions(+), 124 deletions(-)
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply
* [PATCHv4 1/5] submodule.h: add extern keyword to functions
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
As the upcoming series will add a lot of functions to the submodule
header, let's first make the header consistent to the rest of the project
by adding the extern keyword to functions.
As per the CodingGuidelines we try to stay below 80 characters per line,
so adapt all those functions to stay below 80 characters that are already
using more than one line. Those function using just one line are better
kept in one line than breaking them up into multiple lines just for the
goal of staying below the character limit as it makes grepping
for functions easier if they are one liners.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.h | 55 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/submodule.h b/submodule.h
index 6229054b99..61fb610749 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,50 +29,55 @@ struct submodule_update_strategy {
};
#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
-int is_staging_gitmodules_ok(void);
-int update_path_in_gitmodules(const char *oldpath, const char *newpath);
-int remove_path_from_gitmodules(const char *path);
-void stage_updated_gitmodules(void);
-void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
+extern int is_staging_gitmodules_ok(void);
+extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);
+extern int remove_path_from_gitmodules(const char *path);
+extern void stage_updated_gitmodules(void);
+extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
const char *path);
-int submodule_config(const char *var, const char *value, void *cb);
-void gitmodules_config(void);
-int parse_submodule_update_strategy(const char *value,
+extern int submodule_config(const char *var, const char *value, void *cb);
+extern void gitmodules_config(void);
+extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
-const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
-void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
-void show_submodule_summary(FILE *f, const char *path,
+extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
+extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
+extern void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
-void show_submodule_inline_diff(FILE *f, const char *path,
+extern void show_submodule_inline_diff(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset,
const struct diff_options *opt);
-void set_config_fetch_recurse_submodules(int value);
-void check_for_new_submodule_commits(unsigned char new_sha1[20]);
-int fetch_populated_submodules(const struct argv_array *options,
+extern void set_config_fetch_recurse_submodules(int value);
+extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
+extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
int quiet, int max_parallel_jobs);
-unsigned is_submodule_modified(const char *path, int ignore_untracked);
-int submodule_uses_gitfile(const char *path);
-int ok_to_remove_submodule(const char *path);
-int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
- const unsigned char a[20], const unsigned char b[20], int search);
-int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
- struct string_list *needs_pushing);
-int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
-int parallel_submodules(void);
+extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
+extern int submodule_uses_gitfile(const char *path);
+extern int ok_to_remove_submodule(const char *path);
+extern int merge_submodule(unsigned char result[20], const char *path,
+ const unsigned char base[20],
+ const unsigned char a[20],
+ const unsigned char b[20], int search);
+extern int find_unpushed_submodules(unsigned char new_sha1[20],
+ const char *remotes_name,
+ struct string_list *needs_pushing);
+extern int push_unpushed_submodules(unsigned char new_sha1[20],
+ const char *remotes_name);
+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
+extern int parallel_submodules(void);
/*
* Prepare the "env_array" parameter of a "struct child_process" for executing
* a submodule by clearing any repo-specific envirionment variables, but
* retaining any config in the environment.
*/
-void prepare_submodule_repo_env(struct argv_array *out);
+extern void prepare_submodule_repo_env(struct argv_array *out);
#define ABSORB_GITDIR_RECURSE_SUBMODULES (1<<0)
extern void absorb_git_dir_into_superproject(const char *prefix,
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply related
* [PATCHv4 3/5] run-command: add {run,start,finish}_command_or_die
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
In a later patch we want to report the exact command that we run in the
error message. Add a convenient way to the run command API that runs the
given command or reports the exact command as failure.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
run-command.c | 28 ++++++++++++++++++++++++++++
run-command.h | 4 ++++
2 files changed, 32 insertions(+)
diff --git a/run-command.c b/run-command.c
index ca905a9e80..a0587db334 100644
--- a/run-command.c
+++ b/run-command.c
@@ -279,6 +279,17 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
return code;
}
+static void report_and_die(struct child_process *cmd, const char *action)
+{
+ int i;
+ struct strbuf err = STRBUF_INIT;
+ if (cmd->git_cmd)
+ strbuf_addstr(&err, "git ");
+ for (i = 0; cmd->argv[i]; )
+ strbuf_addf(&err, "'%s'", cmd->argv[i]);
+ die(_("could not %s %s"), action, err.buf);
+}
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -546,6 +557,12 @@ int start_command(struct child_process *cmd)
return 0;
}
+void start_command_or_die(struct child_process *cmd)
+{
+ if (start_command(cmd))
+ report_and_die(cmd, "start");
+}
+
int finish_command(struct child_process *cmd)
{
int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
@@ -558,6 +575,11 @@ int finish_command_in_signal(struct child_process *cmd)
return wait_or_whine(cmd->pid, cmd->argv[0], 1);
}
+void finish_command_or_die(struct child_process *cmd)
+{
+ if (finish_command(cmd))
+ report_and_die(cmd, "finish");
+}
int run_command(struct child_process *cmd)
{
@@ -592,6 +614,12 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
return run_command(&cmd);
}
+void run_command_or_die(struct child_process *cmd)
+{
+ if (finish_command(cmd))
+ report_and_die(cmd, "run");
+}
+
#ifndef NO_PTHREADS
static pthread_t main_thread;
static int main_thread_set;
diff --git a/run-command.h b/run-command.h
index dd1c78c28d..e4585885c5 100644
--- a/run-command.h
+++ b/run-command.h
@@ -56,6 +56,10 @@ int finish_command(struct child_process *);
int finish_command_in_signal(struct child_process *);
int run_command(struct child_process *);
+void start_command_or_die(struct child_process *);
+void finish_command_or_die(struct child_process *);
+void run_command_or_die(struct child_process *);
+
/*
* Returns the path to the hook file, or NULL if the hook is missing
* or disabled. Note that this points to static storage that will be
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply related
* [PATCHv4 5/5] rm: absorb a submodules git dir before deletion
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
When deleting a submodule, we need to keep the actual git directory around,
such that we do not lose local changes in there and at a later checkout
of the submodule we don't need to clone it again.
Now that the functionality is available to absorb the git directory of a
submodule, rewrite the checking in git-rm to not complain, but rather
relocate the git directories inside the superproject.
An alternative solution was discussed to have a function
`depopulate_submodule`. That would couple the check for its git directory
and possible relocation before the the removal, such that it is less
likely to miss the check in the future. But the indirection with such
a function added seemed also complex. The reason for that was that this
possible move of the git directory was also implemented in
`ok_to_remove_submodule`, such that this function could truthfully
answer whether it is ok to remove the submodule.
The solution proposed here defers all these checks to the caller.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/rm.c | 79 ++++++++++++++---------------------------------------------
t/t3600-rm.sh | 39 ++++++++++++-----------------
2 files changed, 33 insertions(+), 85 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index fdd7183f61..ed758f7144 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -59,27 +59,9 @@ static void print_error_files(struct string_list *files_list,
}
}
-static void error_removing_concrete_submodules(struct string_list *files, int *errs)
-{
- print_error_files(files,
- Q_("the following submodule (or one of its nested "
- "submodules)\n"
- "uses a .git directory:",
- "the following submodules (or one of their nested "
- "submodules)\n"
- "use a .git directory:", files->nr),
- _("\n(use 'rm -rf' if you really want to remove "
- "it including all of its history)"),
- errs);
- string_list_clear(files, 0);
-}
-
-static int check_submodules_use_gitfiles(void)
+static void submodules_absorb_gitdir_if_needed(const char *prefix)
{
int i;
- int errs = 0;
- struct string_list files = STRING_LIST_INIT_NODUP;
-
for (i = 0; i < list.nr; i++) {
const char *name = list.entry[i].name;
int pos;
@@ -99,12 +81,9 @@ static int check_submodules_use_gitfiles(void)
continue;
if (!submodule_uses_gitfile(name))
- string_list_append(&files, name);
+ absorb_git_dir_into_superproject(prefix, name,
+ ABSORB_GITDIR_RECURSE_SUBMODULES);
}
-
- error_removing_concrete_submodules(&files, &errs);
-
- return errs;
}
static int check_local_mod(struct object_id *head, int index_only)
@@ -120,7 +99,6 @@ static int check_local_mod(struct object_id *head, int index_only)
int errs = 0;
struct string_list files_staged = STRING_LIST_INIT_NODUP;
struct string_list files_cached = STRING_LIST_INIT_NODUP;
- struct string_list files_submodule = STRING_LIST_INIT_NODUP;
struct string_list files_local = STRING_LIST_INIT_NODUP;
no_head = is_null_oid(head);
@@ -218,13 +196,8 @@ static int check_local_mod(struct object_id *head, int index_only)
else if (!index_only) {
if (staged_changes)
string_list_append(&files_cached, name);
- if (local_changes) {
- if (S_ISGITLINK(ce->ce_mode) &&
- !submodule_uses_gitfile(name))
- string_list_append(&files_submodule, name);
- else
- string_list_append(&files_local, name);
- }
+ if (local_changes)
+ string_list_append(&files_local, name);
}
}
print_error_files(&files_staged,
@@ -246,8 +219,6 @@ static int check_local_mod(struct object_id *head, int index_only)
&errs);
string_list_clear(&files_cached, 0);
- error_removing_concrete_submodules(&files_submodule, &errs);
-
print_error_files(&files_local,
Q_("the following file has local modifications:",
"the following files have local modifications:",
@@ -341,6 +312,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
exit(0);
}
+ submodules_absorb_gitdir_if_needed(prefix);
+
/*
* If not forced, the file, the index and the HEAD (if exists)
* must match; but the file can already been removed, since
@@ -357,9 +330,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
oidclr(&oid);
if (check_local_mod(&oid, index_only))
exit(1);
- } else if (!index_only) {
- if (check_submodules_use_gitfiles())
- exit(1);
}
/*
@@ -388,32 +358,20 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
*/
if (!index_only) {
int removed = 0, gitmodules_modified = 0;
- struct strbuf buf = STRBUF_INIT;
for (i = 0; i < list.nr; i++) {
const char *path = list.entry[i].name;
if (list.entry[i].is_submodule) {
- if (is_empty_dir(path)) {
- if (!rmdir(path)) {
- removed = 1;
- if (!remove_path_from_gitmodules(path))
- gitmodules_modified = 1;
- continue;
- }
- } else {
- strbuf_reset(&buf);
- strbuf_addstr(&buf, path);
- if (!remove_dir_recursively(&buf, 0)) {
- removed = 1;
- if (!remove_path_from_gitmodules(path))
- gitmodules_modified = 1;
- strbuf_release(&buf);
- continue;
- } else if (!file_exists(path))
- /* Submodule was removed by user */
- if (!remove_path_from_gitmodules(path))
- gitmodules_modified = 1;
- /* Fallthrough and let remove_path() fail. */
- }
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addstr(&buf, path);
+ if (remove_dir_recursively(&buf, 0))
+ die(_("could not remove '%s'"), path);
+ strbuf_release(&buf);
+
+ removed = 1;
+ if (!remove_path_from_gitmodules(path))
+ gitmodules_modified = 1;
+ continue;
}
if (!remove_path(path)) {
removed = 1;
@@ -422,7 +380,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!removed)
die_errno("git rm: '%s'", path);
}
- strbuf_release(&buf);
if (gitmodules_modified)
stage_updated_gitmodules();
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index bcbb680651..5aa6db584c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -569,26 +569,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
test_cmp expect actual
'
-test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
+test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
git checkout -f master &&
git reset --hard &&
git submodule update &&
(cd submod &&
rm .git &&
cp -R ../.git/modules/sub .git &&
- GIT_WORK_TREE=. git config --unset core.worktree
+ GIT_WORK_TREE=. git config --unset core.worktree &&
+ rm -r ../.git/modules/sub
) &&
- test_must_fail git rm submod &&
- test -d submod &&
- test -d submod/.git &&
- git status -s -uno --ignore-submodules=none >actual &&
- ! test -s actual &&
- test_must_fail git rm -f submod &&
- test -d submod &&
- test -d submod/.git &&
+ git rm submod 2>output.err &&
+ ! test -d submod &&
+ ! test -d submod/.git &&
git status -s -uno --ignore-submodules=none >actual &&
- ! test -s actual &&
- rm -rf submod
+ test -s actual &&
+ test_i18ngrep Migrating output.err
'
cat >expect.deepmodified <<EOF
@@ -667,24 +663,19 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
git submodule update --recursive &&
(cd submod/subsubmod &&
rm .git &&
- cp -R ../../.git/modules/sub/modules/sub .git &&
+ mv ../../.git/modules/sub/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
- test_must_fail git rm submod &&
- test -d submod &&
- test -d submod/subsubmod/.git &&
- git status -s -uno --ignore-submodules=none >actual &&
- ! test -s actual &&
- test_must_fail git rm -f submod &&
- test -d submod &&
- test -d submod/subsubmod/.git &&
+ git rm submod 2>output.err &&
+ ! test -d submod &&
+ ! test -d submod/subsubmod/.git &&
git status -s -uno --ignore-submodules=none >actual &&
- ! test -s actual &&
- rm -rf submod
+ test -s actual &&
+ test_i18ngrep Migrating output.err
'
test_expect_success 'checking out a commit after submodule removal needs manual updates' '
- git commit -m "submodule removal" submod &&
+ git commit -m "submodule removal" submod .gitmodules &&
git checkout HEAD^ &&
git submodule update &&
git checkout -q HEAD^ &&
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply related
* [PATCHv4 2/5] submodule: modernize ok_to_remove_submodule to use argv_array
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
Instead of constructing the NULL terminated array ourselves, we
should make use of the argv_array infrastructure.
While at it, adapt the error messages to reflect the actual invocation.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/submodule.c b/submodule.c
index 45ccfb7ab4..9f0b544ebe 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1023,13 +1023,6 @@ int ok_to_remove_submodule(const char *path)
{
ssize_t len;
struct child_process cp = CHILD_PROCESS_INIT;
- const char *argv[] = {
- "status",
- "--porcelain",
- "-u",
- "--ignore-submodules=none",
- NULL,
- };
struct strbuf buf = STRBUF_INIT;
int ok_to_remove = 1;
@@ -1039,14 +1032,15 @@ int ok_to_remove_submodule(const char *path)
if (!submodule_uses_gitfile(path))
return 0;
- cp.argv = argv;
+ argv_array_pushl(&cp.args, "status", "--porcelain", "-u",
+ "--ignore-submodules=none", NULL);
prepare_submodule_repo_env(&cp.env_array);
cp.git_cmd = 1;
cp.no_stdin = 1;
cp.out = -1;
cp.dir = path;
if (start_command(&cp))
- die("Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s", path);
+ die(_("could not run 'git status --porcelain -u --ignore-submodules=none' in submodule %s"), path);
len = strbuf_read(&buf, cp.out, 1024);
if (len > 2)
@@ -1054,7 +1048,7 @@ int ok_to_remove_submodule(const char *path)
close(cp.out);
if (finish_command(&cp))
- die("'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s", path);
+ die(_("'git status --porcelain -u --ignore-submodules=none' failed in submodule %s"), path);
strbuf_release(&buf);
return ok_to_remove;
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply related
* [PATCHv4 4/5] submodule: add flags to ok_to_remove_submodule
From: Stefan Beller @ 2016-12-19 23:28 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, sandals, David.Turner, Stefan Beller
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
In different contexts the question "Is it ok to delete a submodule?"
may be answered differently.
In 293ab15eea (submodule: teach rm to remove submodules unless they
contain a git directory, 2012-09-26) a case was made that we can safely
ignore ignored untracked files for removal as we explicitely ask for the
removal of the submodule.
In a later patch we want to remove submodules even when the user doesn't
explicitly ask for it (e.g. checking out a tree-ish in which the submodule
doesn't exist). In that case we want to be more careful when it comes
to deletion of untracked files. As of this patch it is unclear how this
will be implemented exactly, so we'll offer flags in which the caller
can specify how the different untracked files ought to be handled.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/rm.c | 3 ++-
submodule.c | 19 +++++++++++++------
submodule.h | 5 ++++-
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 3f3e24eb36..fdd7183f61 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -187,7 +187,8 @@ static int check_local_mod(struct object_id *head, int index_only)
*/
if (ce_match_stat(ce, &st, 0) ||
(S_ISGITLINK(ce->ce_mode) &&
- !ok_to_remove_submodule(ce->name)))
+ !ok_to_remove_submodule(ce->name,
+ SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)))
local_changes = 1;
/*
diff --git a/submodule.c b/submodule.c
index 9f0b544ebe..9aecb8930c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1019,7 +1019,7 @@ int submodule_uses_gitfile(const char *path)
return 1;
}
-int ok_to_remove_submodule(const char *path)
+int ok_to_remove_submodule(const char *path, unsigned flags)
{
ssize_t len;
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1032,23 +1032,30 @@ int ok_to_remove_submodule(const char *path)
if (!submodule_uses_gitfile(path))
return 0;
- argv_array_pushl(&cp.args, "status", "--porcelain", "-u",
+ argv_array_pushl(&cp.args, "status", "--porcelain",
"--ignore-submodules=none", NULL);
+
+ if (flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED)
+ argv_array_push(&cp.args, "-uno");
+ else
+ argv_array_push(&cp.args, "-uall");
+
+ if (!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED))
+ argv_array_push(&cp.args, "--ignored");
+
prepare_submodule_repo_env(&cp.env_array);
cp.git_cmd = 1;
cp.no_stdin = 1;
cp.out = -1;
cp.dir = path;
- if (start_command(&cp))
- die(_("could not run 'git status --porcelain -u --ignore-submodules=none' in submodule %s"), path);
+ start_command_or_die(&cp);
len = strbuf_read(&buf, cp.out, 1024);
if (len > 2)
ok_to_remove = 0;
close(cp.out);
- if (finish_command(&cp))
- die(_("'git status --porcelain -u --ignore-submodules=none' failed in submodule %s"), path);
+ finish_command_or_die(&cp);
strbuf_release(&buf);
return ok_to_remove;
diff --git a/submodule.h b/submodule.h
index 61fb610749..3ed3aa479a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -59,7 +59,10 @@ extern int fetch_populated_submodules(const struct argv_array *options,
int quiet, int max_parallel_jobs);
extern unsigned is_submodule_modified(const char *path, int ignore_untracked);
extern int submodule_uses_gitfile(const char *path);
-extern int ok_to_remove_submodule(const char *path);
+
+#define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<0)
+#define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<1)
+extern int ok_to_remove_submodule(const char *path, unsigned flags);
extern int merge_submodule(unsigned char result[20], const char *path,
const unsigned char base[20],
const unsigned char a[20],
--
2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply related
* Re: [PATCHv4 3/5] run-command: add {run,start,finish}_command_or_die
From: Brandon Williams @ 2016-12-19 23:32 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, sandals, David.Turner
In-Reply-To: <20161219232828.5075-4-sbeller@google.com>
On 12/19, Stefan Beller wrote:
> In a later patch we want to report the exact command that we run in the
> error message. Add a convenient way to the run command API that runs the
> given command or reports the exact command as failure.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> run-command.c | 28 ++++++++++++++++++++++++++++
> run-command.h | 4 ++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index ca905a9e80..a0587db334 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -279,6 +279,17 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
> return code;
> }
>
> +static void report_and_die(struct child_process *cmd, const char *action)
> +{
> + int i;
> + struct strbuf err = STRBUF_INIT;
> + if (cmd->git_cmd)
> + strbuf_addstr(&err, "git ");
> + for (i = 0; cmd->argv[i]; )
Missing the increment of i here.
> + strbuf_addf(&err, "'%s'", cmd->argv[i]);
> + die(_("could not %s %s"), action, err.buf);
> +}
> +
> int start_command(struct child_process *cmd)
> {
> int need_in, need_out, need_err;
> @@ -546,6 +557,12 @@ int start_command(struct child_process *cmd)
> return 0;
> }
>
> +void start_command_or_die(struct child_process *cmd)
> +{
> + if (start_command(cmd))
> + report_and_die(cmd, "start");
> +}
> +
> int finish_command(struct child_process *cmd)
> {
> int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
> @@ -558,6 +575,11 @@ int finish_command_in_signal(struct child_process *cmd)
> return wait_or_whine(cmd->pid, cmd->argv[0], 1);
> }
>
> +void finish_command_or_die(struct child_process *cmd)
> +{
> + if (finish_command(cmd))
> + report_and_die(cmd, "finish");
> +}
>
> int run_command(struct child_process *cmd)
> {
> @@ -592,6 +614,12 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> return run_command(&cmd);
> }
>
> +void run_command_or_die(struct child_process *cmd)
> +{
> + if (finish_command(cmd))
> + report_and_die(cmd, "run");
> +}
> +
> #ifndef NO_PTHREADS
> static pthread_t main_thread;
> static int main_thread_set;
> diff --git a/run-command.h b/run-command.h
> index dd1c78c28d..e4585885c5 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -56,6 +56,10 @@ int finish_command(struct child_process *);
> int finish_command_in_signal(struct child_process *);
> int run_command(struct child_process *);
>
> +void start_command_or_die(struct child_process *);
> +void finish_command_or_die(struct child_process *);
> +void run_command_or_die(struct child_process *);
> +
> /*
> * Returns the path to the hook file, or NULL if the hook is missing
> * or disabled. Note that this points to static storage that will be
> --
> 2.11.0.rc2.53.gb7b3fba.dirty
>
--
Brandon Williams
^ permalink raw reply
* Re: Bug report: $program_name in error message
From: Stefan Beller @ 2016-12-19 23:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Josh Bleecher Snyder, vascomalmeida, git@vger.kernel.org
In-Reply-To: <xmqqfuljod70.fsf@gitster.mtv.corp.google.com>
On Mon, Dec 19, 2016 at 12:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> -- >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>
Looks sensible.
Thanks,
Stefan
> 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
^ permalink raw reply
* Re: [PATCHv4 3/5] run-command: add {run,start,finish}_command_or_die
From: Stefan Beller @ 2016-12-19 23:35 UTC (permalink / raw)
To: Brandon Williams
Cc: Junio C Hamano, git@vger.kernel.org, brian m. carlson,
David Turner
In-Reply-To: <20161219233256.GB41080@google.com>
On Mon, Dec 19, 2016 at 3:32 PM, Brandon Williams <bmwill@google.com> wrote:
>> + strbuf_addstr(&err, "git ");
>> + for (i = 0; cmd->argv[i]; )
>
> Missing the increment of i here.
Doh :(
When writing the code I was undecided between using
an incremental loop "for (i = 0; cmd->argv[i]; i++)" and
a pointer arithmetic thing as "for(;;) {... cmd->argv++;",
and ended up with a broken middle ground.
^ permalink raw reply
* RE: [PATCHv4 0/5] git-rm absorbs submodule git directory before deletion
From: David Turner @ 2016-12-19 23:35 UTC (permalink / raw)
To: 'Stefan Beller', gitster@pobox.com
Cc: git@vger.kernel.org, bmwill@google.com,
sandals@crustytoothpaste.net
In-Reply-To: <20161219232828.5075-1-sbeller@google.com>
Other than Brandon's issue on 3/5 (which I noticed myself but wanted to double-check that my mailer wasn't playing tricks on me and so lost the race to report), this version looks good.
> -----Original Message-----
> From: Stefan Beller [mailto:sbeller@google.com]
> Sent: Monday, December 19, 2016 6:28 PM
> To: gitster@pobox.com
> Cc: git@vger.kernel.org; bmwill@google.com; sandals@crustytoothpaste.net;
> David Turner; Stefan Beller
> Subject: [PATCHv4 0/5] git-rm absorbs submodule git directory before
> deletion
>
> v4:
> * reworded commit messages of the last 2 patches
> * introduced a new patch introducing {run,start,finish}_command_or_die
> * found an existing function in dir.h to use to remove a directory
> which deals gracefully with the cornercases, that Junio pointed out.
>
> v3:
> * removed the patch to enhance ok_to_remove_submodule to absorb the
> submodule
> if needed
> * Removed all the error reporting from git-rm that was related to
> submodule
> git directories not absorbed.
> * instead just absorb the git repositories or let the absorb function die
> with an appropriate error message.
>
> v2:
> * new base where to apply the patch:
> sb/submodule-embed-gitdir merged with sb/t3600-cleanup.
> I got merge conflicts and resolved them this way:
> #@@@ -709,9 -687,10 +687,9 @@@ test_expect_success 'checking out a com
> # git commit -m "submodule removal" submod &&
> # git checkout HEAD^ &&
> # git submodule update &&
> #- git checkout -q HEAD^ 2>actual &&
> #+ git checkout -q HEAD^ &&
> # git checkout -q master 2>actual &&
> # - echo "warning: unable to rmdir submod: Directory not empty"
> >expected &&
> # - test_i18ncmp expected actual &&
> # + test_i18ngrep "^warning: unable to rmdir submod:" actual &&
> # git status -s submod >actual &&
> # echo "?? submod/" >expected &&
> # test_cmp expected actual &&
> #
>
> * improved commit message in "ok_to_remove_submodule: absorb the submodule
> git dir"
> (David Turner offered me some advice on how to write better English off
> list)
> * simplified code in last patch:
> -> dropped wrong comment for fallthrough
> -> moved redundant code out of both bodies of an if-clause.
> * Fixed last patchs commit message to have "or_die" instead of or_dir.
>
> v1:
> The "checkout --recurse-submodules" series got too large to comfortably
> send it out for review, so I had to break it up into smaller series'; this
> is the first subseries, but it makes sense on its own.
>
> This series teaches git-rm to absorb the git directory of a submodule
> instead of failing and complaining about the git directory preventing
> deletion.
>
> It applies on origin/sb/submodule-embed-gitdir.
>
> Any feedback welcome!
>
> Thanks,
> Stefan
>
> Stefan Beller (5):
> submodule.h: add extern keyword to functions
> submodule: modernize ok_to_remove_submodule to use argv_array
> run-command: add {run,start,finish}_command_or_die
> submodule: add flags to ok_to_remove_submodule
> rm: absorb a submodules git dir before deletion
>
> builtin/rm.c | 82 +++++++++++++++---------------------------------------
> -----
> run-command.c | 28 ++++++++++++++++++++ run-command.h | 4 +++
> submodule.c | 27 ++++++++++----------
> submodule.h | 58 ++++++++++++++++++++++++------------------
> t/t3600-rm.sh | 39 +++++++++++-----------------
> 6 files changed, 114 insertions(+), 124 deletions(-)
>
> --
> 2.11.0.rc2.53.gb7b3fba.dirty
^ permalink raw reply
* [PATCH v3] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-19 23:54 UTC (permalink / raw)
To: Junio C Hamano, Jeff King, Johannes Schindelin, Jonathan Tan
Cc: Git mailing list
In-Reply-To: <xmqqbmw7mrg4.fsf@gitster.mtv.corp.google.com>
On Dec 19, 2016, at 15:26, Junio C Hamano wrote:
> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>>> OK. So we do not expect it to fail, but we still do want the side
>>> effect of that function (i.e. accmulation into the field).
>>>
>>> Somebody care to send a final "agreed-upon" version?
>>
>> Yup, here it is:
>
> Thanks.
Whoops. there's an extra paragraph in the commit description that I
meant to remove and, of course, I didn't notice it until I sent the
copy to the list. :(
I don't think a "fixup" or "squash" can replace a description, right?
So here's a replacement patch with the correct description with the
deleted paragrah:
-- >8 --
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.
Since 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, this guarantees a prefix that causes check_header to always
return true.
Therefore replace the assert with an if !check_header + DIE
combination to reflect this.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
Notes:
Please include this PATCH in 2.11.x maint
mailinfo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mailinfo.c b/mailinfo.c
index 2fb3877e..a489d9d0 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -710,7 +710,8 @@ static void flush_inbody_header_accum(struct mailinfo *mi)
{
if (!mi->inbody_header_accum.len)
return;
- assert(check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0));
+ if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0))
+ die("BUG: inbody_header_accum, if not empty, must always contain a valid in-body header");
strbuf_reset(&mi->inbody_header_accum);
}
---
^ permalink raw reply related
* What's cooking in git.git (Dec 2016, #05; Mon, 19)
From: Junio C Hamano @ 2016-12-20 0:21 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
The second (rather large) batch of topics have been merged to
'master'. Please test and catch possible regressions early.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* ah/grammos (2016-12-05) 3 commits
(merged to 'next' on 2016-12-12 at 13ad487b28)
+ clone,fetch: explain the shallow-clone option a little more clearly
+ receive-pack: improve English grammar of denyCurrentBranch message
+ bisect: improve English grammar of not-ancestors message
A few messages have been fixed for their grammatical errors.
* ak/commit-only-allow-empty (2016-12-09) 2 commits
(merged to 'next' on 2016-12-12 at 54188ab23c)
+ commit: remove 'Clever' message for --only --amend
+ commit: make --only --allow-empty work without paths
"git commit --allow-empty --only" (no pathspec) with dirty index
ought to be an acceptable way to create a new commit that does not
change any paths, but it was forbidden, perhaps because nobody
needed it so far.
* bb/unicode-9.0 (2016-12-14) 6 commits
(merged to 'next' on 2016-12-16 at be2531431a)
+ unicode_width.h: update the width tables to Unicode 9.0
+ update_unicode.sh: remove the plane filter
+ update_unicode.sh: automatically download newer definition files
+ update_unicode.sh: pin the uniset repo to a known good commit
+ update_unicode.sh: remove an unnecessary subshell level
+ update_unicode.sh: move it into contrib/update-unicode
The character width table has been updated to match Unicode 9.0
* da/difftool-dir-diff-fix (2016-12-08) 1 commit
(merged to 'next' on 2016-12-12 at fd31a92ad6)
+ difftool: fix dir-diff index creation when in a subdirectory
"git difftool --dir-diff" had a minor regression when started from
a subdirectory, which has been fixed.
* da/mergetool-xxdiff-hotkey (2016-12-11) 1 commit
(merged to 'next' on 2016-12-13 at a08f375c81)
+ mergetools: fix xxdiff hotkeys
The way to specify hotkeys to "xxdiff" that is used by "git
mergetool" has been modernized to match recent versions of xxdiff.
* jb/diff-no-index-no-abbrev (2016-12-08) 1 commit
(merged to 'next' on 2016-12-12 at 959981ef50)
+ diff: handle --no-abbrev in no-index case
"git diff --no-index" did not take "--no-abbrev" option.
* jc/lock-report-on-error (2016-12-07) 3 commits
(merged to 'next' on 2016-12-13 at cb6c07ee92)
+ lockfile: LOCK_REPORT_ON_ERROR
+ hold_locked_index(): align error handling with hold_lockfile_for_update()
+ wt-status: implement opportunisitc index update correctly
Git 2.11 had a minor regression in "merge --ff-only" that competed
with another process that simultanously attempted to update the
index. We used to explain what went wrong with an error message,
but the new code silently failed. The error message has been
resurrected.
* jc/pull-rebase-ff (2016-11-29) 1 commit
(merged to 'next' on 2016-12-16 at c1a0cedd9e)
+ pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
* jc/renormalize-merge-kill-safer-crlf (2016-12-01) 4 commits
(merged to 'next' on 2016-12-12 at 041b834f81)
+ convert: git cherry-pick -Xrenormalize did not work
+ Merge branch 'tb/t0027-raciness-fix' into jc/renormalize-merge-kill-safer-crlf
+ merge-recursive: handle NULL in add_cacheinfo() correctly
+ cherry-pick: demonstrate a segmentation fault
Fix a corner case in merge-recursive regression that crept in
during 2.10 development cycle.
* jk/http-walker-limit-redirect (2016-12-06) 2 commits
(merged to 'next' on 2016-12-12 at 8b58025e3a)
+ http-walker: complain about non-404 loose object errors
+ Merge branch 'ew/http-walker' into jk/http-walker-limit-redirect
(this branch is used by bw/transport-protocol-policy; uses jk/http-walker-limit-redirect-2.9.)
Update the error messages from the dumb-http client when it fails
to obtain loose objects; we used to give sensible error message
only upon 404 but we now forbid unexpected redirects that needs to
be reported with something sensible.
* jk/http-walker-limit-redirect-2.9 (2016-12-06) 5 commits
(merged to 'next' on 2016-12-12 at 3e4bcd7bca)
+ http: treat http-alternates like redirects
+ http: make redirects more obvious
+ remote-curl: rename shadowed options variable
+ http: always update the base URL for redirects
+ http: simplify update_url_from_redirect
(this branch is used by bw/transport-protocol-policy and jk/http-walker-limit-redirect.)
Transport with dumb http can be fooled into following foreign URLs
that the end user does not intend to, especially with the server
side redirects and http-alternates mechanism, which can lead to
security issues. Tighten the redirection and make it more obvious
to the end user when it happens.
* jk/make-tags-find-sources-tweak (2016-12-14) 4 commits
(merged to 'next' on 2016-12-16 at 06d0b0fbfd)
+ Makefile: exclude contrib from FIND_SOURCE_FILES
+ Makefile: match shell scripts in FIND_SOURCE_FILES
+ Makefile: exclude test cruft from FIND_SOURCE_FILES
+ Makefile: reformat FIND_SOURCE_FILES
Update the procedure to generate "tags" for developer support.
* jk/readme-gmane-is-no-more (2016-12-15) 1 commit
(merged to 'next' on 2016-12-16 at 44ad5c5205)
+ README: replace gmane link with public-inbox
* jk/stash-disable-renames-internally (2016-12-06) 1 commit
(merged to 'next' on 2016-12-12 at e2b97aae68)
+ stash: prefer plumbing over git-diff
When diff.renames configuration is on (and with Git 2.9 and later,
it is enabled by default, which made it worse), "git stash"
misbehaved if a file is removed and another file with a very
similar content is added.
* jk/trailers-placeholder-in-pretty (2016-12-11) 2 commits
(merged to 'next' on 2016-12-12 at 57de4e699a)
+ ref-filter: add support to display trailers as part of contents
+ pretty: add %(trailers) format for displaying trailers of a commit message
(this branch uses jt/use-trailer-api-in-commands.)
In addition to %(subject), %(body), "log --pretty=format:..."
learned a new placeholder %(trailers).
* jk/xdiff-drop-xdl-fast-hash (2016-12-06) 1 commit
(merged to 'next' on 2016-12-13 at 914e306217)
+ xdiff: drop XDL_FAST_HASH
Retire the "fast hash" that had disastrous performance issues in
some corner cases.
* js/normalize-path-copy-ceil (2016-12-16) 1 commit
(merged to 'next' on 2016-12-16 at 634ba4debc)
+ normalize_path_copy(): fix pushing to //server/share/dir on Windows
A pathname that begins with "//" or "\\" on Windows is special but
path normalization logic was unaware of it.
* jt/use-trailer-api-in-commands (2016-11-29) 5 commits
(merged to 'next' on 2016-12-12 at da1f140ad4)
+ sequencer: use trailer's trailer layout
+ trailer: have function to describe trailer layout
+ trailer: avoid unnecessary splitting on lines
+ commit: make ignore_non_trailer take buf/len
+ trailer: be stricter in parsing separators
(this branch is used by jk/trailers-placeholder-in-pretty.)
Commands that operate on a log message and add lines to the trailer
blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
"commit -s", have been taught to use the logic of and share the
code with "git interpret-trailer".
* kh/tutorial-grammofix (2016-12-09) 4 commits
(merged to 'next' on 2016-12-13 at a951db78bc)
+ doc: omit needless "for"
+ doc: make the intent of sentence clearer
+ doc: add verb in front of command to run
+ doc: add articles (grammar)
* ld/p4-worktree (2016-12-13) 1 commit
(merged to 'next' on 2016-12-16 at 5210ab9973)
+ git-p4: support git worktrees
"git p4" didn't interact with the internal of .git directory
correctly in the modern "git-worktree"-enabled world.
* lr/doc-fix-cet (2016-12-12) 1 commit
(merged to 'next' on 2016-12-13 at dbc9e07e57)
+ date-formats.txt: Typo fix
* ls/t0021-fixup (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at db652e691a)
+ t0021: minor filter process test cleanup
* ls/travis-update-p4-and-lfs (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 5496caa048)
+ travis-ci: update P4 to 16.2 and GitLFS to 1.5.2 in Linux build
The default Travis-CI configuration specifies newer P4 and GitLFS.
* nd/for-each-ref-ignore-case (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 527cc4f275)
+ tag, branch, for-each-ref: add --ignore-case for sorting and filtering
"git branch --list" and friends learned "--ignore-case" option to
optionally sort branches and tags case insensitively.
* nd/rebase-forget (2016-12-11) 1 commit
(merged to 'next' on 2016-12-12 at 50b5d28af4)
+ rebase: add --quit to cleanup rebase, leave everything else untouched
"git rebase" learned "--quit" option, which allows a user to
remove the metadata left by an earlier "git rebase" that was
manually aborted without using "git rebase --abort".
* rj/git-version-gen-do-not-force-abbrev (2016-12-06) 1 commit
(merged to 'next' on 2016-12-12 at e37970c3f5)
+ GIT-VERSION-GEN: do not force abbreviation length used by 'describe'
A minor build update.
* sb/t3600-cleanup (2016-12-12) 2 commits
(merged to 'next' on 2016-12-13 at e06e6e702f)
+ t3600: slightly modernize style
(merged to 'next' on 2016-12-12 at d9996af5e8)
+ t3600: remove useless redirect
Code cleanup.
* sb/unpack-trees-grammofix (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 29e536f590)
+ unpack-trees: fix grammar for untracked files in directories
--------------------------------------------------
[New Topics]
* gv/p4-multi-path-commit-fix (2016-12-19) 1 commit
- git-p4: fix multi-path changelist empty commits
"git p4" that tracks multile p4 paths imported a single changelist
that touches files in these multiple paths as one commit, followed
by many empty commits. This has been fixed.
Will merge to 'next'.
* js/mingw-isatty (2016-12-18) 1 commit
- winansi_isatty(): fix when Git is used from CMD
Update the isatty() emulation for Windows to make it interact
better with the cmd.exe console.
Waiting for an ack.
* jt/mailinfo-fold-in-body-headers (2016-12-19) 1 commit
- mailinfo.c: move side-effects outside of assert
Fix for NDEBUG builds.
Will merge to 'next'.
* ls/p4-lfs (2016-12-18) 1 commit
- git-p4: add diff/merge properties to .gitattributes for GitLFS files
Update GitLFS integration with "git p4".
Waiting for an ack.
* ls/p4-path-encoding (2016-12-18) 1 commit
- git-p4: fix git-p4.pathEncoding for removed files
When "git p4" imports changelist that removes paths, it failed to
convert pathnames when the p4 used encoding different from the one
used on the Git side. This has been corrected.
Waiting for an ack.
* mh/fast-import-notes-fix-new (2016-12-19) 1 commit
- fast-import: properly fanout notes when tree is imported
"git fast-import" sometimes mishandled while rebalancing notes
tree, which has been fixed.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* jc/retire-compaction-heuristics (2016-11-02) 3 commits
- SQUASH???
- SQUASH???
- diff: retire the original experimental "compaction" heuristics
Waiting for a reroll.
* jc/abbrev-autoscale-config (2016-11-01) 1 commit
- config.abbrev: document the new default that auto-scales
Waiting for a reroll.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
- exclude: do not respect symlinks for in-tree .gitignore
- attr: do not respect symlinks for in-tree .gitattributes
- exclude: convert "check_index" into a flags field
- attr: convert "macro_ok" into a flags field
- add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* nd/worktree-move (2016-11-28) 11 commits
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
. copy.c: convert copy_file() to copy_dir_recursively()
. copy.c: style fix
. copy.c: convert bb_(p)error_msg to error(_errno)
. copy.c: delete unused code in copy_file()
. copy.c: import copy_file() from busybox
"git worktree" learned move and remove subcommands.
Reported to break builds on Windows.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* bw/pathspec-cleanup (2016-12-14) 16 commits
- pathspec: rename prefix_pathspec to init_pathspec_item
- pathspec: small readability changes
- pathspec: create strip submodule slash helpers
- pathspec: create parse_element_magic helper
- pathspec: create parse_long_magic function
- pathspec: create parse_short_magic function
- pathspec: factor global magic into its own function
- pathspec: simpler logic to prefix original pathspec elements
- pathspec: always show mnemonic and name in unsupported_magic
- pathspec: remove unused variable from unsupported_magic
- pathspec: copy and free owned memory
- pathspec: remove the deprecated get_pathspec function
- ls-tree: convert show_recursive to use the pathspec struct interface
- dir: convert fill_directory to use the pathspec struct interface
- dir: remove struct path_simplify
- mv: remove use of deprecated 'get_pathspec()'
Code clean-up in the pathspec API.
Waiting for the (hopefully) final round of review before 'next'.
* cp/merge-continue (2016-12-15) 4 commits
(merged to 'next' on 2016-12-19 at 8ba0094f45)
+ merge: mark usage error strings for translation
+ merge: ensure '--abort' option takes no arguments
+ completion: add --continue option for merge
+ merge: add '--continue' option as a synonym for 'git commit'
"git merge --continue" has been added as a synonym to "git commit"
to conclude a merge that has stopped due to conflicts.
Will merge to 'master'.
* jk/parseopt-usage-msg-opt (2016-12-14) 1 commit
(merged to 'next' on 2016-12-19 at c488c7c6e1)
+ parse-options: print "fatal:" before usage_msg_opt()
The function usage_msg_opt() has been updated to say "fatal:"
before the custom message programs give, when they want to die
with a message about wrong command line options followed by the
standard usage string.
Will merge to 'master'.
* ld/p4-compare-dir-vs-symlink (2016-12-18) 1 commit
- git-p4: avoid crash adding symlinked directory
"git p4" misbehaved when swapping a directory and a symbolic link.
Will merge to 'next'.
* js/prepare-sequencer-more (2016-12-14) 34 commits
- sequencer (rebase -i): write out the final message
- sequencer (rebase -i): write the progress into files
- sequencer (rebase -i): show the progress
- sequencer (rebase -i): suggest --edit-todo upon unknown command
- sequencer (rebase -i): show only failed cherry-picks' output
- sequencer (rebase -i): show only failed `git commit`'s output
- run_command_opt(): optionally hide stderr when the command succeeds
- sequencer (rebase -i): differentiate between comments and 'noop'
- sequencer (rebase -i): implement the 'drop' command
- sequencer (rebase -i): allow rescheduling commands
- sequencer (rebase -i): respect strategy/strategy_opts settings
- sequencer (rebase -i): respect the rebase.autostash setting
- sequencer (rebase -i): run the post-rewrite hook, if needed
- sequencer (rebase -i): record interrupted commits in rewritten, too
- sequencer (rebase -i): copy commit notes at end
- sequencer (rebase -i): set the reflog message consistently
- sequencer (rebase -i): refactor setting the reflog message
- sequencer (rebase -i): allow fast-forwarding for edit/reword
- sequencer (rebase -i): implement the 'reword' command
- sequencer (rebase -i): leave a patch upon error
- sequencer (rebase -i): update refs after a successful rebase
- sequencer (rebase -i): the todo can be empty when continuing
- sequencer (rebase -i): skip some revert/cherry-pick specific code path
- sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
- sequencer (rebase -i): allow continuing with staged changes
- sequencer (rebase -i): write an author-script file
- sequencer (rebase -i): implement the short commands
- sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
- sequencer (rebase -i): write the 'done' file
- sequencer (rebase -i): learn about the 'verbose' mode
- sequencer (rebase -i): implement the 'exec' command
- sequencer (rebase -i): implement the 'edit' command
- sequencer (rebase -i): implement the 'noop' command
- sequencer: support a new action: 'interactive rebase'
The sequencer has further been extended in preparation to act as a
back-end for "rebase -i".
Waiting for review.
* jk/index-pack-wo-repo-from-stdin (2016-12-16) 4 commits
(merged to 'next' on 2016-12-19 at 9a88221347)
+ index-pack: skip collision check when not in repository
+ t: use nongit() function where applicable
+ index-pack: complain when --stdin is used outside of a repo
+ t5000: extract nongit function to test-lib-functions.sh
"git index-pack --stdin" needs an access to an existing repository,
but "git index-pack file.pack" to generate an .idx file that
corresponds to a packfile does not.
Will merge to 'master'.
* lt/shortlog-by-committer (2016-12-16) 2 commits
(merged to 'next' on 2016-12-19 at 555976fc0a)
+ shortlog: test and document --committer option
+ shortlog: group by committer information
"git shortlog" learned "--committer" option to group commits by
committer, instead of author.
Will merge to 'master'.
* bw/realpath-wo-chdir (2016-12-12) 4 commits
- real_path: have callers use real_pathdup and strbuf_realpath
- real_path: create real_pathdup
- real_path: convert real_path_internal to strbuf_realpath
- real_path: resolve symlinks by hand
(this branch is used by bw/grep-recurse-submodules.)
The implementation of "real_path()" was to go there with chdir(2)
and call getcwd(3), but this obviously wouldn't be usable in a
threaded environment. Rewrite it to manually resolve relative
paths including symbolic links in path components.
Will merge to 'next'.
* jk/quote-env-path-list-component (2016-12-13) 4 commits
(merged to 'next' on 2016-12-16 at d2cd6008b9)
+ t5547-push-quarantine: run the path separator test on Windows, too
+ tmp-objdir: quote paths we add to alternates
+ alternates: accept double-quoted paths
+ Merge branch 'jk/alt-odb-cleanup' into jk/quote-env-path-list-component
A recent update to receive-pack to make it easier to drop garbage
objects made it clear that GIT_ALTERNATE_OBJECT_DIRECTORIES cannot
have a pathname with a colon in it (no surprise!), and this in turn
made it impossible to push into a repository at such a path. This
has been fixed by introducing a quoting mechanism used when
appending such a path to the colon-separated list.
Will merge to 'master'.
* nd/shallow-fixup (2016-12-07) 6 commits
(merged to 'next' on 2016-12-13 at 1a3edb8bce)
+ shallow.c: remove useless code
+ shallow.c: bit manipulation tweaks
+ shallow.c: avoid theoretical pointer wrap-around
+ shallow.c: make paint_alloc slightly more robust
+ shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory pools
+ shallow.c: rename fields in paint_info to better express their purposes
Code cleanup in shallow boundary computation.
Will merge to 'master'.
* sb/sequencer-abort-safety (2016-12-14) 6 commits
(merged to 'next' on 2016-12-16 at ec71fb1217)
+ Revert "sequencer: remove useless get_dir() function"
(merged to 'next' on 2016-12-13 at 6107e43d65)
+ sequencer: remove useless get_dir() function
+ sequencer: make sequencer abort safer
+ t3510: test that cherry-pick --abort does not unsafely change HEAD
+ am: change safe_to_abort()'s not rewinding error into a warning
+ am: fix filename in safe_to_abort() error message
Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back
to where cherry-pick started while picking multiple changes, when
the cherry-pick stopped to ask for help from the user, and the user
did "git reset --hard" to a different commit in order to re-attempt
the operation.
Will merge to 'master'.
* jk/difftool-in-subdir (2016-12-11) 4 commits
- difftool: rename variables for consistency
- difftool: chdir as early as possible
- difftool: sanitize $workdir as early as possible
- difftool: fix dir-diff index creation when in a subdirectory
Even though an fix was attempted in Git 2.9.3 days, but running
"git difftool --dir-diff" from a subdirectory never worked. This
has been fixed.
Will merge to 'next'.
* vs/submodule-clone-nested-submodules-alternates (2016-12-12) 1 commit
(merged to 'next' on 2016-12-13 at 8a317ab745)
+ submodule--helper: set alternateLocation for cloned submodules
"git clone --reference $there --recurse-submodules $super" has been
taught to guess repositories usable as references for submodules of
$super that are embedded in $there while making a clone of the
superproject borrow objects from $there; extend the mechanism to
also allow submodules of these submodules to borrow repositories
embedded in these clones of the submodules embedded in the clone of
the superproject.
Will merge to 'master'.
* ls/filter-process (2016-12-18) 2 commits
(merged to 'next' on 2016-12-19 at 5ed29656db)
+ t0021: fix flaky test
(merged to 'next' on 2016-12-12 at 8ed1f9eb02)
+ docs: warn about possible '=' in clean/smudge filter process values
Doc update.
Will merge to 'master'.
* js/difftool-builtin (2016-11-28) 2 commits
- difftool: implement the functionality in the builtin
- difftool: add a skeleton for the upcoming builtin
Rewrite a scripted porcelain "git difftool" in C.
What's the doneness of this topic?
* sb/push-make-submodule-check-the-default (2016-11-29) 2 commits
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
+ submodule add: extend force flag to add existing repos
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will cook in 'next'.
* sb/submodule-embed-gitdir (2016-12-12) 6 commits
- submodule: add absorb-git-dir function
- move connect_work_tree_and_git_dir to dir.h
- worktree: check if a submodule uses worktrees
- test-lib-functions.sh: teach test_commit -C <dir>
- submodule helper: support super prefix
- submodule: use absolute path for computing relative path connecting
A new submodule helper "git submodule embedgitdirs" to make it
easier to move embedded .git/ directory for submodules in a
superproject to .git/modules/ (and point the latter with the former
that is turned into a "gitdir:" file) has been added.
Will merge to 'next'.
* bw/grep-recurse-submodules (2016-12-16) 7 commits
- grep: search history of moved submodules
- grep: enable recurse-submodules to work on <tree> objects
- grep: optionally recurse into submodules
- grep: add submodules as a grep source type
- submodules: load gitmodules file from commit sha1
- submodules: add helper to determine if a submodule is initialized
- submodules: add helper to determine if a submodule is populated
(this branch uses bw/realpath-wo-chdir.)
"git grep" learns to optionally recurse into submodules
Will merge to 'next'.
* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
(merged to 'next' on 2016-12-05 at 3ea70d01af)
+ upload-pack: optionally allow fetching any sha1
+ remote-curl: don't hang when a server dies before any output
Originally merged to 'next' on 2016-11-21
When the http server gives an incomplete response to a smart-http
rpc call, it could lead to client waiting for a full response that
will never come. Teach the client side to notice this condition
and abort the transfer.
An improvement counterproposal has failed.
cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
Will cook in 'next'.
* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
(merged to 'next' on 2016-12-05 at 9a2b5bd1a9)
+ doc: mention transfer data leaks in more places
Originally merged to 'next' on 2016-11-16
Doc update on fetching and pushing.
Will cook in 'next'.
* jc/compression-config (2016-11-15) 1 commit
(merged to 'next' on 2016-12-05 at 323769ca07)
+ compression: unify pack.compression configuration parsing
Originally merged to 'next' on 2016-11-23
Compression setting for producing packfiles were spread across
three codepaths, one of which did not honor any configuration.
Unify these so that all of them honor core.compression and
pack.compression variables the same way.
Will cook in 'next'.
* mm/gc-safety-doc (2016-11-16) 1 commit
(merged to 'next' on 2016-12-05 at 031ecc1886)
+ git-gc.txt: expand discussion of races with other processes
Originally merged to 'next' on 2016-11-17
Doc update.
Will cook in 'next'.
* kn/ref-filter-branch-list (2016-12-08) 20 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
- ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
- ref-filter: rename the 'strip' option to 'lstrip'
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
- for-each-ref: do not segv with %(HEAD) on an unborn branch
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
What's the doneness of the topic? I recall discussing die vs empty
and also saw a "squash this in when you reroll", but I lost track.
* bw/transport-protocol-policy (2016-12-15) 6 commits
(merged to 'next' on 2016-12-19 at 166168205c)
+ http: respect protocol.*.allow=user for http-alternates
+ transport: add from_user parameter to is_transport_allowed
+ http: create function to get curl allowed protocols
+ transport: add protocol policy config option
+ http: always warn if libcurl version is too old
+ lib-proto-disable: variable name fix
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.
Will merge to 'master'.
* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
(merged to 'next' on 2016-12-05 at 432f9469a7)
+ fetch: do not redundantly calculate tag refmap
Originally merged to 'next' on 2016-11-16
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.
Will cook in 'next'.
* sb/submodule-config-cleanup (2016-11-22) 3 commits
(merged to 'next' on 2016-12-05 at 658b8764bf)
+ submodule-config: clarify parsing of null_sha1 element
+ submodule-config: rename commit_sha1 to treeish_name
+ submodule config: inline config_from_{name, path}
Originally merged to 'next' on 2016-11-23
Minor code clean-up.
Will cook in 'next'.
* jc/push-default-explicit (2016-10-31) 2 commits
(merged to 'next' on 2016-12-05 at d63f3777af)
+ push: test pushing ambiguously named branches
+ push: do not use potentially ambiguous default refspec
Originally merged to 'next' on 2016-11-01
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.
Will cook in 'next'.
* jc/git-open-cloexec (2016-11-02) 3 commits
- sha1_file: stop opening files with O_NOATIME
- git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
- git_open(): untangle possible NOATIME and CLOEXEC interactions
The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
opens has been simplified.
We may want to drop the tip one.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
Will discard.
This may not be needed, given that update-index has a similar
option.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
Will discard.
The whole premise feels wrong.
* tb/convert-stream-check (2016-10-27) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Will discard.
Retracted.
cf. <20161102071646.GA5094@tb-raspi>
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Waiting for review.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-11-11) 35 commits
. completion: clone can initialize specific submodules
. clone: add --init-submodule=<pathspec> switch
. submodule update: add `--init-default-path` switch
. pathspec: allow escaped query values
. pathspec: allow querying for attributes
. pathspec: move prefix check out of the inner loop
. pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
The parts near the tip about pathspec would need to work better
with bw/pathspec-cleanup topic and has been dropped for now.
* va/i18n-perl-scripts (2016-12-14) 16 commits
(merged to 'next' on 2016-12-19 at ec800aba9f)
+ i18n: difftool: mark warnings for translation
+ i18n: send-email: mark composing message for translation
+ i18n: send-email: mark string with interpolation for translation
+ i18n: send-email: mark warnings and errors for translation
+ i18n: send-email: mark strings for translation
+ i18n: add--interactive: mark status words for translation
+ i18n: add--interactive: remove %patch_modes entries
+ i18n: add--interactive: mark edit_hunk_manually message for translation
+ i18n: add--interactive: i18n of help_patch_cmd
+ i18n: add--interactive: mark patch prompt for translation
+ i18n: add--interactive: mark plural strings
+ i18n: clean.c: match string with git-add--interactive.perl
+ i18n: add--interactive: mark strings with interpolation for translation
+ i18n: add--interactive: mark simple here-documents for translation
+ i18n: add--interactive: mark strings for translation
+ Git.pm: add subroutines for commenting lines
Porcelain scripts written in Perl are getting internationalized.
Will merge to 'master'.
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-12-05 at fb549caa12)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Originally merged to 'next' on 2016-09-28
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will cook in 'next'.
* sg/fix-versioncmp-with-common-suffix (2016-12-08) 8 commits
- versioncmp: generalize version sort suffix reordering
- squash! versioncmp: use earliest-longest contained suffix to determine sorting order
- versioncmp: use earliest-longest contained suffix to determine sorting order
- versioncmp: cope with common part overlapping with prerelease suffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for review.
cf. <20161208142401.1329-1-szeder.dev@gmail.com>
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
^ permalink raw reply
* Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file
From: Junio C Hamano @ 2016-12-20 1:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1612191800530.54750@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio,
>
> On Thu, 15 Dec 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>
>> > + strbuf_addstr(&buf, "GIT_AUTHOR_NAME='");
>> > + while (*message && *message != '\n' && *message != '\r')
>> > + if (skip_prefix(message, " <", &message))
>> > + break;
>> > + else if (*message != '\'')
>> > + strbuf_addch(&buf, *(message++));
>> > + else
>> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
>> > + strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
>> > + while (*message && *message != '\n' && *message != '\r')
>> > + if (skip_prefix(message, "> ", &message))
>> > + break;
>> > + else if (*message != '\'')
>> > + strbuf_addch(&buf, *(message++));
>> > + else
>> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
>>
>> Aren't these reading from an in-core commit object?
>>
>> If so, it should use split_ident_line() for consistency with other
>> parts of the system to do this parsing. We should also already have
>> a helper for simple shell-quoting in quote.c and you would want to
>> use that instead of open coding like this.
>
> We keep coming back to the same argument. You want this quoting/dequoting
> to be turned into a full-fledged parser. And I keep pointing out that the
> code here does not *need* to parse but only construct an environment
> block.
I am afraid you are mis-reading me. I see a code that _READS_ some
data format, for which we already have a parser, and then write out
things based on what it read. I do not want you to make anything
into a full-fledged parser---I just do not want to see an ad-hoc
reader and instead the code to USE existing parser.
^ permalink raw reply
* Re: [PATCHv8 6/6] submodule: add absorb-git-dir function
From: Duy Nguyen @ 2016-12-20 1:39 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kYPEiUGXR-qTbbHzaeOwbHH88mdx7GP8QX2Ff1bypcrwQ@mail.gmail.com>
On Tue, Dec 20, 2016 at 1:15 AM, Stefan Beller <sbeller@google.com> wrote:
> 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?
That's when you pray the UNIX gods that recovery steps don't fail :-)
This is why I don't _suggest_ to do things but just wonder about it.
In theory though, if we keep recovery to dead simple operations (e.g.
a series of rename() and nothing else) then it's less likely to fail.
I'll look at the new patches when I get home.
--
Duy
^ permalink raw reply
* Re: [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
From: Max Kirillov @ 2016-12-20 5:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Karsten Blees, git, Johannes Sixt
In-Reply-To: <xmqqtw9zmvjq.fsf@gitster.mtv.corp.google.com>
On Mon, Dec 19, 2016 at 01:57:29PM -0800, Junio C Hamano wrote:
> Max, I see this is a resend from a few days ago
Sorry about resend. For some reason I don't get the list
copy (might be some clever duplicate elimination in my
forwardings), and marc.info seems to be slow to update, so I
had no indication the first message got into list. Now I see
they are there in the other archive.
PS: probably http://vger.kernel.org/vger-lists.html#git
should be updated because there is no archive at gmane
anymore.
--
Max
^ permalink raw reply
* Re: [PATCH 1/5] check-ref-format: Refactor out check_one_ref_format
From: Michael Haggerty @ 2016-12-20 6:57 UTC (permalink / raw)
To: Ian Jackson; +Cc: git
In-Reply-To: <22615.56956.698915.2223@chiark.greenend.org.uk>
On 12/19/2016 02:19 PM, Ian Jackson wrote:
> Michael Haggerty writes ("Re: [PATCH 1/5] check-ref-format: Refactor out check_one_ref_format"):
>> On 11/04/2016 08:13 PM, Ian Jackson wrote:
>>> +static int check_one_ref_format(const char *refname)
> ...
>> This function needs to `return 0` if it gets to the end.
>
> Indeed it does. I'm kind of surprised my compiler didn't spot that.
Our build system has a `DEVELOPER` option [1] that turns on lots of
errors and warnings, and you should turn it on if you haven't already:
echo DEVELOPER=1 >>config.mak
What exactly it catches depends on what compiler you are using, but it
definitely helps if you are using gcc, and I think also if you are using
clang.
Michael
[1]
https://github.com/git/git/blob/6610af872f6494a061780ec738c8713a034b848b/Documentation/CodingGuidelines#L174-L177
^ permalink raw reply
* Re: [PATCH 0/5] git check-ref-format --stdin --report-errors
From: Michael Haggerty @ 2016-12-20 7:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ian Jackson, git
In-Reply-To: <xmqqlgvbpyku.fsf@gitster.mtv.corp.google.com>
On 12/19/2016 07:23 PM, Junio C Hamano wrote:
> 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?
Of course that would be the ideal. Let's think about what it would look
like. Given that the very purpose of the program is to decide whether
its inputs are reasonable reference names or not, it would be important
to make it bulletproof:
* It could be fed some ugly garbage
* It could be used for security-relevant checks
One obvious choice would be to use NUL separators, but that would make
the output mostly unreadable to humans.
Another would be to use LF to terminate each line of output, like
ok TAB refs/heads/foo LF
bad TAB refs/heads/bad SP name@@.lock LF
For the LF-terminated `--stdin` input, this should be unambiguous.
However, it wouldn't necessarily work for arguments passed in via the
command line, for for slight variations on `--stdin` like if we were to
add a `-z` option to allow the input to be NUL-terminated.
The 100% solution would probably be to support language-specific
quoting, like the `--shell`/`--perl`/`--python`/`--tcl` options accepted
by `for-each-ref`, probably with a fifth option for NUL-terminated
output. And it should probably also support a `-z` option to make its
input NUL-separated. Pretty much all of the infrastructure is already
there in `quote.h` and `quote.c`, and the option-parsing could be
cribbed from `builtin/for-each-ref.c`, so it wouldn't even be *that*
much work to implement.
Michael
^ 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