* [PATCH 0/9] The final building block for a faster rebase -i
From: Johannes Schindelin @ 2016-09-02 16:22 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
This patch series reimplements the expensive pre- and post-processing of
the todo script in C.
And it concludes the work I did to accelerate rebase -i.
Johannes Schindelin (9):
rebase -i: generate the script via rebase--helper
rebase -i: remove useless indentation
rebase -i: do not invent onelines when expanding/collapsing SHA-1s
rebase -i: also expand/collapse the SHA-1s via the rebase--helper
t3404: relax rebase.missingCommitsCheck tests
rebase -i: check for missing commits in the rebase--helper
rebase -i: skip unnecessary picks using the rebase--helper
t3415: test fixup with wrapped oneline
rebase -i: rearrange fixup/squash lines using the rebase--helper
builtin/rebase--helper.c | 29 ++-
git-rebase--interactive.sh | 362 ++++-------------------------
sequencer.c | 514 ++++++++++++++++++++++++++++++++++++++++++
sequencer.h | 7 +
t/t3404-rebase-interactive.sh | 22 +-
t/t3415-rebase-autosquash.sh | 16 +-
6 files changed, 614 insertions(+), 336 deletions(-)
Based-On: rebase--helper at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git rebase--helper
Published-As: https://github.com/dscho/git/releases/tag/rebase-i-extra-v1
Fetch-It-Via: git fetch https://github.com/dscho/git rebase-i-extra-v1
--
2.9.3.windows.3
base-commit: 4c39918f42eb8228ea4241073f86f2ac851f4636
^ permalink raw reply
* [PATCH 2/9] rebase -i: remove useless indentation
From: Johannes Schindelin @ 2016-09-02 16:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1472833365.git.johannes.schindelin@gmx.de>
The commands used to be indented, and it is nice to look at, but when we
transform the SHA-1s, the indentation is removed. So let's do away with it.
For the moment, at least: when we will use the upcoming rebase--helper
to transform the SHA-1s, we *will* keep the indentation and can
reintroduce it. Yet, to be able to validate the rebase--helper against
the output of the current shell script version, we need to remove the
extra indentation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-rebase--interactive.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 01c9fec..5df5850 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -146,13 +146,13 @@ reschedule_last_action () {
append_todo_help () {
gettext "
Commands:
- p, pick = use commit
- r, reword = use commit, but edit the commit message
- e, edit = use commit, but stop for amending
- s, squash = use commit, but meld into previous commit
- f, fixup = like \"squash\", but discard this commit's log message
- x, exec = run command (the rest of the line) using shell
- d, drop = remove commit
+p, pick = use commit
+r, reword = use commit, but edit the commit message
+e, edit = use commit, but stop for amending
+s, squash = use commit, but meld into previous commit
+f, fixup = like \"squash\", but discard this commit's log message
+x, exec = run command (the rest of the line) using shell
+d, drop = remove commit
These lines can be re-ordered; they are executed from top to bottom.
" | git stripspace --comment-lines >>"$todo"
--
2.9.3.windows.3
^ permalink raw reply related
* [PATCH 1/9] rebase -i: generate the script via rebase--helper
From: Johannes Schindelin @ 2016-09-02 16:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1472833365.git.johannes.schindelin@gmx.de>
The first step of an interactive rebase is to generate the so-called "todo
script", to be stored in the state directory as "git-rebase-todo" and to
be edited by the user.
Originally, we adjusted the output of `git log <options>` using a simple
sed script. Over the course of the years, the code became more
complicated. We now use shell scripting to edit the output of `git log`
conditionally, depending whether to keep "empty" commits (i.e. commits
that do not change any files).
On platforms where shell scripting is not native, this can be a serious
drag. And it opens the door for incompatibilities between platforms when
it comes to shell scripting or to Unix-y commands.
Let's just re-implement the todo script generation in plain C, using the
revision machinery directly.
This is substantially faster, improving the speed relative to the
shell script version of the interactive rebase from 2x to 3x on Windows.
Note that the rearrange_squash() function in git-rebase--interactive
relied on the fact that we set the "format" variable to the config setting
rebase.instructionFormat. Relying on a side effect like this is no good,
hence we explicitly perform that assignment (possibly again) in
rearrange_squash().
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase--helper.c | 8 +++++++-
git-rebase--interactive.sh | 44 +++++++++++++++++++++++---------------------
sequencer.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
sequencer.h | 2 ++
4 files changed, 76 insertions(+), 22 deletions(-)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index ca1ebb2..821058d 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -11,15 +11,19 @@ static const char * const builtin_rebase_helper_usage[] = {
int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts = REPLAY_OPTS_INIT;
+ int keep_empty = 0;
enum {
- CONTINUE = 1, ABORT
+ CONTINUE = 1, ABORT, MAKE_SCRIPT
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
+ OPT_BOOL(0, "keep-empty", &keep_empty, N_("keep empty commits")),
OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", &command, N_("abort rebase"),
ABORT),
+ OPT_CMDMODE(0, "make-script", &command,
+ N_("make rebase script"), MAKE_SCRIPT),
OPT_END()
};
@@ -36,5 +40,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
return !!sequencer_continue(&opts);
if (command == ABORT && argc == 1)
return !!sequencer_remove_state(&opts);
+ if (command == MAKE_SCRIPT && argc > 1)
+ return !!sequencer_make_script(keep_empty, stdout, argc, argv);
usage_with_options(builtin_rebase_helper_usage, options);
}
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 022766b..01c9fec 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -775,6 +775,7 @@ collapse_todo_ids() {
# each log message will be re-retrieved in order to normalize the
# autosquash arrangement
rearrange_squash () {
+ format=$(git config --get rebase.instructionFormat)
# extract fixup!/squash! lines and resolve any referenced sha1's
while read -r pick sha1 message
do
@@ -1203,26 +1204,27 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-format=$(git config --get rebase.instructionFormat)
-# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
-git rev-list $merges_option --format="%m%H ${format:-%s}" \
- --reverse --left-right --topo-order \
- $revisions ${restrict_revision+^$restrict_revision} | \
- sed -n "s/^>//p" |
-while read -r sha1 rest
-do
-
- if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
- then
- comment_out="$comment_char "
- else
- comment_out=
- fi
+if test t != "$preserve_merges"
+then
+ git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+ $revisions ${restrict_revision+^$restrict_revision} >"$todo"
+else
+ format=$(git config --get rebase.instructionFormat)
+ # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
+ git rev-list $merges_option --format="%m%H ${format:-%s}" \
+ --reverse --left-right --topo-order \
+ $revisions ${restrict_revision+^$restrict_revision} | \
+ sed -n "s/^>//p" |
+ while read -r sha1 rest
+ do
+
+ if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit $sha1
+ then
+ comment_out="$comment_char "
+ else
+ comment_out=
+ fi
- if test t != "$preserve_merges"
- then
- printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
- else
if test -z "$rebase_root"
then
preserve=t
@@ -1241,8 +1243,8 @@ do
touch "$rewritten"/$sha1
printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
fi
- fi
-done
+ done
+fi
# Watch for commits that been dropped by --cherry-pick
if test t = "$preserve_merges"
diff --git a/sequencer.c b/sequencer.c
index c0c6661..43e078a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2347,3 +2347,47 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
strbuf_release(&sob);
}
+
+int sequencer_make_script(int keep_empty, FILE *out,
+ int argc, const char **argv)
+{
+ char *format = "%s";
+ struct pretty_print_context pp = {0};
+ struct strbuf buf = STRBUF_INIT;
+ struct rev_info revs;
+ struct commit *commit;
+
+ init_revisions(&revs, NULL);
+ revs.verbose_header = 1;
+ revs.max_parents = 1;
+ revs.cherry_pick = 1;
+ revs.limited = 1;
+ revs.reverse = 1;
+ revs.right_only = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.topo_order = 1;
+
+ revs.pretty_given = 1;
+ git_config_get_string("rebase.instructionFormat", &format);
+ get_commit_format(format, &revs);
+ pp.fmt = revs.commit_format;
+ pp.output_encoding = get_log_output_encoding();
+
+ if (setup_revisions(argc, argv, &revs, NULL) > 1)
+ return error("make_script: unhandled options");
+
+ if (prepare_revision_walk(&revs) < 0)
+ return error("make_script: error preparing revisions");
+
+ while ((commit = get_revision(&revs))) {
+ strbuf_reset(&buf);
+ if (!keep_empty && is_original_commit_empty(commit))
+ strbuf_addf(&buf, "%c ", comment_line_char);
+ strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
+ pretty_print_commit(&pp, commit, &buf);
+ strbuf_addch(&buf, '\n');
+ fputs(buf.buf, out);
+ }
+ strbuf_release(&buf);
+ return 0;
+}
diff --git a/sequencer.h b/sequencer.h
index fd2a719..bc524be 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -58,6 +58,8 @@ int sequencer_remove_state(struct replay_opts *opts);
int sequencer_commit(const char *defmsg, struct replay_opts *opts,
int allow_empty, int edit, int amend,
int cleanup_commit_message);
+int sequencer_make_script(int keep_empty, FILE *out,
+ int argc, const char **argv);
extern const char sign_off_header[];
--
2.9.3.windows.3
^ permalink raw reply related
* git add -p—splitting hunks, limit is too large
From: Beau Martinez @ 2016-09-02 14:36 UTC (permalink / raw)
To: git
Hi git developers and community,
I'd like to inquire as to why `git add -p` can only split hunks so
much. The limit is too large; why can't you split until each hunk is
only a line? I often have to run `edit` and split them manually
myself.
I'd like to contribute a patch to change it, although my C is rusty.
Are there resources that will help me to do this?
Thank you for your time.
Beau
^ permalink raw reply
* Re: Git in Outreachy December-March?
From: Christian Couder @ 2016-09-02 14:34 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20160902090247.b5gtui75hiwococc@sigill.intra.peff.net>
On Fri, Sep 2, 2016 at 11:02 AM, Jeff King <peff@peff.net> wrote:
>
> I'm happy to act as admin. We will need a few things:
>
> - to arrange funding for the stipend. GitHub offered to cover this
> last time, and if we are interested, I can see if this is still the
> case. We can also cover it out of the Git project money.
>
> - mentor volunteers. This is similar in scope to GSoC, but I don't
> want to just assume that people who volunteered for GSoC would still
> be available
I would be happy to co-mentor as well as for GSoC.
> - projects. We can pull from the ideas that were not selected for the
> 2016 GSoC, but we may need to update or add to it.
Yeah, maybe we could also update the micro-project page
(https://github.com/git/git.github.io/blob/master/SoC-2016-Microprojects.md).
^ permalink raw reply
* Re: [PATCH 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-09-02 14:22 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git, Junio C Hamano
In-Reply-To: <1472818007.4680.53.camel@kaarsemaker.net>
Hi Dennis,
On Fri, 2 Sep 2016, Dennis Kaarsemaker wrote:
> On vr, 2016-09-02 at 09:13 +0200, Johannes Schindelin wrote:
>
> > As Git for Windows does not ship with translations (for multiple
> > reasons), it would not be a regression.
>
> I'm confused, how does "git for windows does not ship with
> translations" translate to "this is not a regression"? Is this patch
> series only meant to be for git for windows and not go into git.git
> itself?
Oh, I thought I had clarified my plan... The timeline is:
- I submit the remaining rebase--helper patch series for review (last week
and this one),
- I publish a preview of Git for Windows v2.10.0 that already uses these
patches (done: https://github.com/git-for-windows/git/releases/tag/v2.9.3.windows.3)
- once upstream Git v2.10.0 is released (possibly today, after my work
hours), I perform a final "Git garden shears" run (read: rebase Git for
Windows' patches, retaining the branch structure) on top of v2.10.0 and
release Git for Windows v2.10.0, tagged as v2.10.0.windows.1 in
https://github.com/git-for-windows/git (due to time zone differences
relative to Junio, the most likely time for this release would be
some time around noon tomorrow, given that the release engineering takes
roughly 2-4 hours, running unsupervised for the most part).
- as far as Git for Windows is concerned, l10n is not really an issue yet:
the installer is released without any localizations.
- After releasing Git for Windows v2.10.0, I will pay a lot of attention
to feedback. Not only to hear a lot of praise, but also to catch any
possible regressions. Not that I expect anything dramatic to happen
because I really tested this as thoroughly as I can: not a single one of
my interactive rebases since mid May has been performed without
involving the rebase--helper. In the three cases where I *did* find a
regression, I solved it immediately, of course.
- After releasing Git for Windows v2.10.0, I will have a nice beer. Or
three.
- Then I will leisurely try to address the l10n issues.
- Then, I will send out the current iterations of the patch series that
are in flight.
- I have the entire week to address concerns with Git for Windows as well
as with the patch series (where the former takes precedence, of course).
- The second half of September, I will relax from this marathon that
started in early February. Meaning: I will be mostly offline.
I hope this clarifies why I am not so concerned about some issues such as
translation, or commit messages, or grammar, and more so about others,
such as incorrect code.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 00/22] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-09-02 13:56 UTC (permalink / raw)
To: Jakub Narębski
Cc: git, Junio C Hamano, Dennis Kaarsemaker, Johannes Sixt
In-Reply-To: <ced4a190-6a79-e608-ca0b-3815267c5f93@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11176 bytes --]
Hi Kuba,
On Fri, 2 Sep 2016, Jakub Narębski wrote:
> W dniu 29.08.2016 o 10:03, Johannes Schindelin pisze:
>
> > This patch series marks the '4' in the countdown to speed up rebase -i
> > by implementing large parts in C. It is based on the `libify-sequencer`
> > patch series that I submitted last week.
>
> Which of those got reviewed (and perhaps accepted), and which of those
> needs review still? What is subject of their cover letter?
Most of the patch series I sent before last week got accepted. Only one
got rejected, IIRC, and replaced by a better solution (3727318 (Merge
branch 'jk/test-send-sh-x-trace-elsewhere', 2016-05-17)).
The patch series I submitted as part of my rebase--helper work that were
accepted:
b232439 (Merge branch 'js/t3404-typofix', 2016-05-17)
7b02771 (Merge branch 'js/perf-rebase-i', 2016-05-23)
3437017 (Merge branch 'js/perf-on-apple', 2016-07-06)
62e5e83 (Merge branch 'js/find-commit-subject-ignore-leading-blanks', 2016-07-11)
c510926 (Merge branch 'js/sign-empty-commit-fix', 2016-07-13)
6c35952 (Merge branch 'js/t3404-grammo-fix', 2016-07-13)
63641fb (Merge branch 'js/log-to-diffopt-file', 2016-07-19)
3d55eea (Merge branch 'js/am-call-theirs-theirs-in-fallback-3way', 2016-07-19)
c97268c (Merge branch 'js/rebase-i-tests', 2016-07-28)
1a5f1a3 (Merge branch 'js/am-3-merge-recursive-direct', 2016-08-10)
You will note that I broke out a couple of patch series that do not
strictly have anything to do with the rebase--helper, such as
perf-on-apple. Nevertheless, they were part of a 99-strong patch series
that was my initial working rebase--helper, which I have used ever since
to perform all of my interactive rebases.
There are still a couple of patch series in flight. Let me list them by
the tags created by my mail-patch-series.sh script:
https://github.com/dscho/git/releases/tag/libify-sequencer-v2
https://github.com/dscho/git/releases/tag/require-clean-work-tree-v1
https://github.com/dscho/git/releases/tag/prepare-sequencer-v1
https://github.com/dscho/git/releases/tag/sequencer-i-v1
https://github.com/dscho/git/releases/tag/rebase--helper-v1
These tags all contain links to the cover letter as stored on
public-inbox.org, identified by the Message-ID.
Please note that the first four of this batch of five already saw
substantial work-after-review, thanks in part to your helpful comments.
You may appreciate the fact that a link of the form
https://github.com/dscho/git/compare/libify-sequencer-v2...libify-sequencer
shows you where I am at, although it cannot give you a real interdiff
because I rebased to a newer version of upstream/master in the meantime.
Finally, there is one last patch series that I did not yet submit: the
'rebase-i-extra' patch series. However, as I continuously update the
overall 'interactive-rebase' branch thicket (and have done so since the
very beginning of my work on the rebase--helper), it is relatively easy to
see what is left:
https://github.com/dscho/git/compare/rebase--helper...interactive-rebase
BTW thanks for making me dig out all of this information (it did take a
while to uncover it...), as I am so totally going to use that in a blog
post.
> > The reason to split these two patch series is simple: to keep them at a
> > sensible size.
>
> That's good.
Thanks. I really try to be sensible with other people's time.
Even more so after being so offended by the talk at the most recent Git
Merge that stated that some people deliberately waste contributors' time
because they value their own time so much more. I am *really* offended by
that.
As a maintainer of Git for Windows, I do everything in my power to strike
a sensible balance between how much time I spend on improving the software
and how much time I ask others to do so.
> > The two patch series after that are much smaller: a two-patch "series"
> > that switches rebase -i to use the sequencer (except with --root or
> > --preserve-merges), and a couple of patches to move several pretty
> > expensive script processing steps to C (think: autosquash).
>
> I can understand --preserve-merges, but what is the problem with --root?
The problem with --root is that it *creates* an initial commit. It is
empty, and will be amended. It would most likely not be a lot of work, but
I really wanted this work to be incremental, focusing on the most
important aspects first.
In fact, I do hope that somebody with the need for --root will take the
baton and run with it.
> > The end game of this patch series is a git-rebase--helper that makes
> > rebase -i 5x faster on Windows (according to t/perf/p3404). Travis
> > says that even MacOSX and Linux benefit (4x and 3x, respectively).
>
> So do I understand correctly that end goal for *this* series is to move
> most of processing to git-rebase--helper, but full builtin-ification
> (and retiring git-rebase.sh to contrib/examples/) would have to wait for
> later?
Oh yes!
Retiring git-rebase.sh is *far, far, far* in the future. We really missed
the boat a *looooong* time ago to turn this from a hacky shell script into
a proper C builtin.
There is so much more to do before git-rebase.sh can be retired.
For starters, git-rebase.sh is actually just a glorified command-line
option parser and front-end to git-rebase--am.sh,
git-rebase--interactive.sh and git-rebase--merge.sh.
To retire it, those three shell scripts need to be *completely* built-in
first.
(Actually, for the --preserve-merges case, I could imagine that we simply
refactor it into its own shell script and call that from a builtin
git-rebase, until we retire --preserve-merges, but that's a couple of
years down the road.)
So the first goal would be to retire git-rebase--interactive.sh. For that
to happen, --root needs to be supported first. Then the --preserve-merges
stuff needs to be refactored into its own shell script. And then the
command-line option parsing needs to be moved to rebase--helper, too. And
*then* git-rebase--interactive.sh can be retired.
As I stated earlier, my hope is that the rebase--helper work is only an
initial step, opening the door for other contributors to tackle
independent parts of making git-rebase a builtin.
> [...]
>
> I'd like here to summarize the discussion (my review, Dennis review,
> Johannes Sixt and Junio comments).
>
> If there are no comments, it means no problems or minor changes.
Please keep in mind that my current state (local, and pushed to my GitHub
repository) has advanced substantially. I am reluctant to send it out yet
because I still need to send out rebase-i-extra first, so that it gets
*some* visibility before v2.10.0.
> > Johannes Schindelin (22):
> > sequencer: use static initializers for replay_opts
> There is no need for putting zeros in static initializer. Commit
> message expanded.
>
> > sequencer: use memoized sequencer directory path
> > sequencer: avoid unnecessary indirection
> > sequencer: future-proof remove_sequencer_state()
> Leftover unrelated chunk removed.
>
> > sequencer: allow the sequencer to take custody of malloc()ed data
> Is introducing new *_entrust() mechanism (which needs docs, at least
> as comments) worth it, instead of just strdup everything and free?
> If it is: naming of function parameter + example in commit message.
>
> > sequencer: release memory that was allocated when reading options
> See above.
>
> > sequencer: future-proof read_populate_todo()
> Possibly mention which functions were not future-proofed because
> of planned for the subsequent patch full rewrite.
Note that this commit is about read_populate_todo(), not about
save_todo(). So I do not think that we should mention anything in this
commit's message about other functions that may be rewritten instead of
being future-proofed..
> > sequencer: remove overzealous assumption
> Overzealous assumptions, or a worthy check? Perhaps just remove check
> for rebase -i in future commit, and keep test. Perhaps remove test
> temporarily.
As mentioned earlier, I bit the bullet and reimplemented that logic.
Mostly to fend off more comments in this direction.
> > sequencer: completely revamp the "todo" script parsing
> This removes check; it should return if it was worthy. Some discussion
> about eager versus lazy parsing of commits, but IMHO it should be left
> for later, if considered worth it.
Again, it was reintroduced. The test to check for the overzealous
assumption was not removed, and it passes, to prove that I did it right.
> > sequencer: avoid completely different messages for different actions
> Fix l10n or drop (and not introduce lego translation).
>
> > sequencer: get rid of the subcommand field
> > sequencer: refactor the code to obtain a short commit name
> Explain reason behind this change in the commit mesage.
>
> > sequencer: remember the onelines when parsing the todo file
> Lazy or eager again; "exec", "noop" and --preserve-merges.
>
> > sequencer: prepare for rebase -i's commit functionality
> Add helper function, possibly extract helper function. Rephrase block
> comment.
>
> "[PATCH] am: refactor read_author_script()" from Junio.
>
> > sequencer: introduce a helper to read files written by scripts
> Perhaps add why not use open + strbuf_getline to commit message...
>
> > sequencer: prepare for rebase -i's GPG settings
> Possibly fixes bug. Use *_entrust() or strdup to not leak memory
> (and to not crash when freeing memory).
>
> > sequencer: allow editing the commit message on a case-by-case basis
> Enhance the commit message.
>
> > sequencer: support amending commits
> > sequencer: support cleaning up commit messages
> > sequencer: remember do_recursive_merge()'s return value
> > sequencer: left-trim the lines read from the script
> > sequencer: refactor write_message()
> Enhance the commit message. Quote path in messages while at it.
Apart from the l10n issues, I think I addressed them all locally, and
pushed the result out to my GitHub repository, although I plan to send out
additional iterations only after releasing Git for Windows v2.10.0.
> > Based-On: libify-sequencer at https://github.com/dscho/git
> > Fetch-Base-Via: git fetch https://github.com/dscho/git libify-sequencer
> > Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v1
> > Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v1
>
> An unrelated question: Dscho, how are you generating above lines?
It's the `mail-patch-series.sh` script, in conjunction with setting the
config variable mail.publishremoteto to point to my GitHub remote. You can
find the `mail-patch-series.sh` script here:
https://github.com/dscho/mail-patch-series
I probably forgot to adjust the README to reflect the most recent changes
(such as the `--basedon` feature)... PRs welcome [*1*].
Ciao,
Dscho
Footnote *1*:
https://raw.githubusercontent.com/dscho/images/master/i-can-haz-pull-request.png
^ permalink raw reply
* Re: [GIT PULL] l10n updates for 2.10.0 round 2
From: Jiang Xin @ 2016-09-02 13:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Trần Ngọc Quân, Git List
In-Reply-To: <xmqqr393t4k5.fsf@gitster.mtv.corp.google.com>
Hi Junio,
Another update comes, please pull.
The following changes since commit 5b18e70009487bb156cac18546d6f91105338f4c:
A few more fixes before the final 2.10 (2016-08-31 10:21:05 -0700)
are available in the git repository at:
git://github.com/git-l10n/git-po tags/l10n-2.10.0-rnd2.2
for you to fetch changes up to e8e349249c86550d3505c4abfac28caf3d13df46:
Merge branch 'master' of https://github.com/vnwildman/git
(2016-09-02 21:29:48 +0800)
----------------------------------------------------------------
l10n-2.10.0-rnd2.2
----------------------------------------------------------------
Jiang Xin (1):
Merge branch 'master' of https://github.com/vnwildman/git
Trần Ngọc Quân (1):
l10n: Updated Vietnamese translation for v2.10.0-rc2 (2757t)
po/vi.po | 691 +++++++++++++++++++++++++++++----------------------------------
1 file changed, 317 insertions(+), 374 deletions(-)
2016-09-02 10:32 GMT+08:00 Junio C Hamano <gitster@pobox.com>:
> Trần Ngọc Quân <vnwildman@gmail.com> writes:
>
>> On 31/08/2016 21:14, Jiang Xin wrote:
>>> Hi Junio,
>>>
>>> Would you please pull the following git l10n updates.
>> Please wait! Jiang Xin probably missing pull my one commit[1].
>>
>> [1]
>> <https://github.com/vnwildman/git/commit/800d88e2b3dde41ebf34e2e00955bba892419555>
>
> Jiang, I do not mind another update from you before the final.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2016-09-02 12:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, Git, Junio C Hamano
In-Reply-To: <CACBZZX56fjJZydnBrWUYtU6V3xyQyaLL4MYzVVF0yD4dRdducw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]
Hi Ævar,
On Fri, 2 Sep 2016, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > The biggest problem with Strawberry Perl is that it is virtually
> > impossible to build the Subversion-Perl bindings using the Git for
> > Windows SDK when using Strawberry Perl.
> >
> > Which pretty much precludes it from being used in Git for Windows.
> >
> > And then there are the path issues... Git's Perl scripts are pretty
> > certain that they live in a POSIX-y environment. Which MSYS2 Perl
> > provides. Strawberry Perl not.
>
> This might be me missing the point, and I'm really just trying to be
> helpful here and make "prove" work for you because it's awesome, but
> as far as just you running this for development purposes does any of
> this SVN stuff matter? I.e. you can build Git itself not with
> Strawberry, but just use Strawberry to get a working copy of "prove".
Yes, the SVN stuff matters, because of the many t9*svn* tests (which, BTW
take a substantial time to run). So if I run the test suite, I better do
it with a perl.exe in the PATH that can run the SVN tests. Otherwise I
might just as well not bother with running the entire test suite...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Dennis Kaarsemaker @ 2016-09-02 12:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1609012009070.129229@virtualbox>
On vr, 2016-09-02 at 09:13 +0200, Johannes Schindelin wrote:
> As Git for Windows does not ship with translations (for multiple
> reasons), it would not be a regression.
I'm confused, how does "git for windows does not ship with
translations" translate to "this is not a regression"? Is this patch
series only meant to be for git for windows and not go into git.git
itself?
D.
^ permalink raw reply
* Re: [PATCH 00/22] Prepare the sequencer for the upcoming rebase -i patches
From: Jakub Narębski @ 2016-09-02 11:41 UTC (permalink / raw)
To: Johannes Schindelin, git
Cc: Junio C Hamano, Dennis Kaarsemaker, Johannes Sixt
In-Reply-To: <cover.1472457609.git.johannes.schindelin@gmx.de>
W dniu 29.08.2016 o 10:03, Johannes Schindelin pisze:
> This patch series marks the '4' in the countdown to speed up rebase -i
> by implementing large parts in C. It is based on the `libify-sequencer`
> patch series that I submitted last week.
Which of those got reviewed (and perhaps accepted), and which of those
needs review still? What is subject of their cover letter?
>
> The patches in this series merely prepare the sequencer code for the
> next patch series that actually teaches the sequencer to run an
> interactive rebase.
>
> The reason to split these two patch series is simple: to keep them at a
> sensible size.
That's good.
>
> The two patch series after that are much smaller: a two-patch "series"
> that switches rebase -i to use the sequencer (except with --root or
> --preserve-merges), and a couple of patches to move several pretty
> expensive script processing steps to C (think: autosquash).
I can understand --preserve-merges, but what is the problem with --root?
>
> The end game of this patch series is a git-rebase--helper that makes
> rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says
> that even MacOSX and Linux benefit (4x and 3x, respectively).
So do I understand correctly that end goal for *this* series is to move
most of processing to git-rebase--helper, but full builtin-ification
(and retiring git-rebase.sh to contrib/examples/) would have to wait
for later?
[...]
I'd like here to summarize the discussion (my review, Dennis review,
Johannes Sixt and Junio comments).
If there are no comments, it means no problems or minor changes.
> Johannes Schindelin (22):
> sequencer: use static initializers for replay_opts
There is no need for putting zeros in static initializer. Commit
message expanded.
> sequencer: use memoized sequencer directory path
> sequencer: avoid unnecessary indirection
> sequencer: future-proof remove_sequencer_state()
Leftover unrelated chunk removed.
> sequencer: allow the sequencer to take custody of malloc()ed data
Is introducing new *_entrust() mechanism (which needs docs, at least
as comments) worth it, instead of just strdup everything and free?
If it is: naming of function parameter + example in commit message.
> sequencer: release memory that was allocated when reading options
See above.
> sequencer: future-proof read_populate_todo()
Possibly mention which functions were not future-proofed because
of planned for the subsequent patch full rewrite.
> sequencer: remove overzealous assumption
Overzealous assumptions, or a worthy check? Perhaps just remove check
for rebase -i in future commit, and keep test. Perhaps remove test
temporarily.
> sequencer: completely revamp the "todo" script parsing
This removes check; it should return if it was worthy. Some discussion
about eager versus lazy parsing of commits, but IMHO it should be left
for later, if considered worth it.
> sequencer: avoid completely different messages for different actions
Fix l10n or drop (and not introduce lego translation).
> sequencer: get rid of the subcommand field
> sequencer: refactor the code to obtain a short commit name
Explain reason behind this change in the commit mesage.
> sequencer: remember the onelines when parsing the todo file
Lazy or eager again; "exec", "noop" and --preserve-merges.
> sequencer: prepare for rebase -i's commit functionality
Add helper function, possibly extract helper function. Rephrase block
comment.
"[PATCH] am: refactor read_author_script()" from Junio.
> sequencer: introduce a helper to read files written by scripts
Perhaps add why not use open + strbuf_getline to commit message...
> sequencer: prepare for rebase -i's GPG settings
Possibly fixes bug. Use *_entrust() or strdup to not leak memory
(and to not crash when freeing memory).
> sequencer: allow editing the commit message on a case-by-case basis
Enhance the commit message.
> sequencer: support amending commits
> sequencer: support cleaning up commit messages
> sequencer: remember do_recursive_merge()'s return value
> sequencer: left-trim the lines read from the script
> sequencer: refactor write_message()
Enhance the commit message. Quote path in messages while at it.
HTH
>
> builtin/commit.c | 2 +-
> builtin/revert.c | 42 ++-
> sequencer.c | 573 +++++++++++++++++++++++++++-------------
> sequencer.h | 27 +-
> t/t3510-cherry-pick-sequence.sh | 11 -
> 5 files changed, 428 insertions(+), 227 deletions(-)
>
> Based-On: libify-sequencer at https://github.com/dscho/git
> Fetch-Base-Via: git fetch https://github.com/dscho/git libify-sequencer
> Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v1
An unrelated question: Dscho, how are you generating above lines?
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests
From: Ævar Arnfjörð Bjarmason @ 2016-09-02 10:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1608311702440.129229@virtualbox>
On Wed, Aug 31, 2016 at 5:05 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Ævar,
>
> On Wed, 31 Aug 2016, Ævar Arnfjörð Bjarmason wrote:
>
>> I haven't used it myself (or any Windows thing) but people say good
>> things about http://strawberryperl.com
>
> Ah yes. This comes up frequently. Many a Git for Windows user pointed me
> into that direction.
>
> The biggest problem with Strawberry Perl is that it is virtually
> impossible to build the Subversion-Perl bindings using the Git for Windows
> SDK when using Strawberry Perl.
>
> Which pretty much precludes it from being used in Git for Windows.
>
> And then there are the path issues... Git's Perl scripts are pretty
> certain that they live in a POSIX-y environment. Which MSYS2 Perl
> provides. Strawberry Perl not.
This might be me missing the point, and I'm really just trying to be
helpful here and make "prove" work for you because it's awesome, but
as far as just you running this for development purposes does any of
this SVN stuff matter? I.e. you can build Git itself not with
Strawberry, but just use Strawberry to get a working copy of "prove".
^ permalink raw reply
* Re: Git in Outreachy December-March?
From: Pranit Bauva @ 2016-09-02 9:35 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20160902090247.b5gtui75hiwococc@sigill.intra.peff.net>
Probably off-topic.
On Fri, Sep 2, 2016 at 2:32 PM, Jeff King <peff@peff.net> wrote:
> As some of you may recall, we signed up to participate in Outreachy for
> the May-August session, but did not end up selecting an intern. The
> original thread with details is here:
>
> http://public-inbox.org/git/20160308224625.GA29922@sigill.intra.peff.net/
>
> There's another session that runs from December to March. If we want to
> participate, we need to sign up in the next few days.
>
> I'm happy to act as admin. We will need a few things:
>
> - to arrange funding for the stipend. GitHub offered to cover this
> last time, and if we are interested, I can see if this is still the
> case. We can also cover it out of the Git project money.
>
> - mentor volunteers. This is similar in scope to GSoC, but I don't
> want to just assume that people who volunteered for GSoC would still
> be available
>
> - projects. We can pull from the ideas that were not selected for the
> 2016 GSoC, but we may need to update or add to it.
I have a few friends who too did GSoC along with me but in different
orgs. Their orgs have a separate channel (slack or IRC) for
GSoC/Outreachy communications. In that channel is that all potential
mentors are added and students too are pointed to it. In that channel
the very basic doubts are covered. Let's say I am online and probably
the student's mentor is currently unavailable, so he/she can post a
trivial doubt there for someone to respond quickly. It takes off a
little load from the list and the mentor as well. I am aware that
there exists a channel named #git-devel but unfortunately its not
really active. I will be wiling to help other students with their
early days!
These days a lot of my fellow students don't really use IRC for
communication and see you can see there were really less number of
people inquiring about GSoC in #git-devel. We can prefer slack or any
other alternative.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: Should "git symbolic-ref -d HEAD" be forbidden?
From: Andreas Schwab @ 2016-09-02 9:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqpoonuy4n.fsf@gitster.mtv.corp.google.com>
On Sep 01 2016, Junio C Hamano <gitster@pobox.com> wrote:
> I think we should.
>
> t1401 expects to be able to, but if you really do it:
>
> $ cd /tmp
> $ git init throwaway
> $ cd throwaway
> $ git symbolic-ref -d HEAD
>
> the setup machinery considers that you are no longer in a working
> tree that is controlled by a repository at .git/ because .git/ is
> no longer a valid repository, so you cannot even do
>
> $ git symbolic-ref HEAD refs/heads/master
>
> to recover.
git init recovers it, though.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: bug: 'core.logallrefupdates' is not set by default in non-bare repository
From: Jeff King @ 2016-09-02 9:11 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: doak, git@vger.kernel.org
In-Reply-To: <1472806914.4680.50.camel@kaarsemaker.net>
On Fri, Sep 02, 2016 at 11:01:54AM +0200, Dennis Kaarsemaker wrote:
> Well, 'git init' is a valid operation to run inside an existing repo to
> reinitialize some bits, so we definitely need to not ignore the config
> once we're sure we're not creating a new repo.
Good point, I hadn't considered re-initializing.
For the follow-up patch I sent, where we check
startup_info->have_repository, I think the right thing would probably be
to call setup_git_directory() after seeing we are in a re-init case.
Probably even the "gently" form, as I think you can "re-init" a
partially corrupted repository.
> > > @@ -500,7 +506,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> > > * and we know shared_repository should always be 0;
> > > * but just in case we play safe.
> > > */
> > > - saved = get_shared_repository();
> > > set_shared_repository(0);
> > > switch (safe_create_leading_directories_const(argv[0])) {
> > > case SCLD_OK:
> > I don't know if anybody cares about being able to set core.sharedRepository
> > from ~/.gitconfig. It didn't work until v2.9.0 anyway (when I moved it
> > out of the repository-format check), but it seems like you _should_ be
> > able to set it and have it Just Work.
> >
> > And in that case, this "we know shared_repository should always be 0" is
> > not true, and we would want to keep doing the save/set-to-0/restore
> > dance here.
>
> We don't need to save if we throw away the cache below.
Yeah, you're right. Though I somehow lost my train of thought between
the two paragraphs. I was thinking that we would want to actually
respect the ~/.gitconfig setting for sharedrepository. Which would
actually mean _dropping_ the save/zero/restore entirely, and just using
the value we get from the config. But I guess the point here is to avoid
s_c_l_d creating "shared" leading directories that are outside any
repository. I could see an argument either way on whether that is the
right thing to do when core.sharedRepository is set in ~/.gitconfig.
-Peff
^ permalink raw reply
* Git in Outreachy December-March?
From: Jeff King @ 2016-09-02 9:02 UTC (permalink / raw)
To: git
As some of you may recall, we signed up to participate in Outreachy for
the May-August session, but did not end up selecting an intern. The
original thread with details is here:
http://public-inbox.org/git/20160308224625.GA29922@sigill.intra.peff.net/
There's another session that runs from December to March. If we want to
participate, we need to sign up in the next few days.
I'm happy to act as admin. We will need a few things:
- to arrange funding for the stipend. GitHub offered to cover this
last time, and if we are interested, I can see if this is still the
case. We can also cover it out of the Git project money.
- mentor volunteers. This is similar in scope to GSoC, but I don't
want to just assume that people who volunteered for GSoC would still
be available
- projects. We can pull from the ideas that were not selected for the
2016 GSoC, but we may need to update or add to it.
-Peff
^ permalink raw reply
* Re: bug: 'core.logallrefupdates' is not set by default in non-bare repository
From: Dennis Kaarsemaker @ 2016-09-02 9:01 UTC (permalink / raw)
To: Jeff King; +Cc: doak, git@vger.kernel.org
In-Reply-To: <20160902080416.jmrctu3onfmylmeq@sigill.intra.peff.net>
On vr, 2016-09-02 at 04:04 -0400, Jeff King wrote:
> On Wed, Aug 31, 2016 at 05:32:33PM +0200, Dennis Kaarsemaker wrote:
>
> >
> > >
> > > We may need to do something like turn off the
> > > need_shared_repository_from_config in init-db, since I think it would
> > > not want to ever read from the default config sources in most of its
> > > code-paths (OTOH, it should in theory respect core.sharedRepository
> > > in ~/.gitconfig, so maybe there is another more elegant way of
> > > handling this).
> > I would go even further and say that git init should completely ignore
> > the config of a repository you happen to be in when creating a new
> > repository.
> Hmm. I'd think we would already be avoiding that, because we shouldn't
> be calling setup_git_directory(). But some of the lazy-loaded setup is a
> bit overzealous, and we blindly look at ".git/config". If we try the
> same operation from a subdir of an existing repo, we _don't_ end up
> confused. Eek.
Yikes. Didnt' dig that deep, but that sounds wrong :)
> So I actually wonder if that is the root of the bug. In your patch, you
> disable config reading when we chdir to the new repo:
>
> >
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index 3a45f0b..d0fd3dc 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -493,6 +493,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> > int mkdir_tried = 0;
> > retry:
> > if (chdir(argv[0]) < 0) {
> > + /*
> > + * We're creating a new repository. If we're already in another
> > + * repository, ignore its config
> > + */
> > + ignore_repo_config = 1;
> > + git_config_clear();
> But I think we should go further and avoid ever looking at the original
> repository in the first place. I.e., I would say that "git init" should
> never ever behave differently if run in an existing repo versus outside
> of one.
Well, 'git init' is a valid operation to run inside an existing repo to
reinitialize some bits, so we definitely need to not ignore the config
once we're sure we're not creating a new repo.
> So really we ought to be setting ignore_repo_config from the very start
> of cmd_init(), and then re-enabling it once we are "inside" the new
> repo. The git_config_clear() should in theory come once we are
> "inside", as well; we may have cached system/global config, and
> need to flush so we read them anew along with the new local config.
That's why I git_config_clear() twice.
> OTOH, since there shouldn't be anything interesting in the new
> repo-level config, I'm not sure that's really necessary. The rest of
> "init" can probably proceed without caring.
Except when running 'git init' to re-init existing repo.
> I also wonder if there are other things besides config which might
> accidentally read from .git (because they call git_pathdup(), and it
> just blindly looks in ".git" if nobody called setup_git_directory()). So
> it would be nice to have some flag for "do not ever lazy-call
> setup_git_env; we do not care about any repository". But I think that's
> ahrd; functions like git_pathdup() are always expected to return _some_
> value, so what would they say? The best we could do is return
> "/does-not-exist/" or something, but that is awfully hacky.
>
> >
> > @@ -500,7 +506,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> > * and we know shared_repository should always be 0;
> > * but just in case we play safe.
> > */
> > - saved = get_shared_repository();
> > set_shared_repository(0);
> > switch (safe_create_leading_directories_const(argv[0])) {
> > case SCLD_OK:
> I don't know if anybody cares about being able to set core.sharedRepository
> from ~/.gitconfig. It didn't work until v2.9.0 anyway (when I moved it
> out of the repository-format check), but it seems like you _should_ be
> able to set it and have it Just Work.
>
> And in that case, this "we know shared_repository should always be 0" is
> not true, and we would want to keep doing the save/set-to-0/restore
> dance here.
We don't need to save if we throw away the cache below.
> > @@ -524,6 +528,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> > } else if (0 < argc) {
> > usage(init_db_usage[0]);
> > }
> > +
> > + need_shared_repository_from_config = 1;
> > + ignore_repo_config = 0;
> > + git_config_clear();
> This is the part I think we could actually skip. The only thing we might
> not have loaded is the "config" we just wrote to the new repository. But
> I don't think we have to care about what is in it.
We do, because this is also called for existing repos.
> > diff --git a/config.c b/config.c
> > index 0dfed68..2df0189 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -1304,7 +1304,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
> > ret += git_config_from_file(fn, user_config, data);
> >
> > current_parsing_scope = CONFIG_SCOPE_REPO;
> > - if (repo_config && !access_or_die(repo_config, R_OK, 0))
> > + if (repo_config && !ignore_repo_config && !access_or_die(repo_config, R_OK, 0))
> > ret += git_config_from_file(fn, repo_config, data);
> We probably want to intercept the call to git_pathdup() earlier than
> this, if the point is not to touch any of the lazy-load setup_git_dir()
> stuff at all. The effect is the same for config, but I think it makes
> sense to have as little effect as possible.
Thought about doing that, but didn't know what side-effects that would
have.
> So here's the minimal fix that seems to work for me:
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 3a45f0b..56e7b9a 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -484,6 +484,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> OPT_END()
> };
>
> + ignore_repo_config = 1;
> +
> argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
>
> if (real_git_dir && !is_absolute_path(real_git_dir))
> diff --git a/cache.h b/cache.h
> index b780a91..13b78e4 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1582,6 +1582,13 @@ enum config_origin_type {
> CONFIG_ORIGIN_CMDLINE
> };
>
> +/*
> + * If non-zero, git_config() will avoid any attempt to find the repo config;
> + * this is useful for programs like git-init that might look at config before
> + * actually setting up the new repository.
> + */
> +extern int ignore_repo_config;
> +
> typedef int (*config_fn_t)(const char *, const char *, void *);
> extern int git_default_config(const char *, const char *, void *);
> extern int git_config_from_file(config_fn_t fn, const char *, void *);
> diff --git a/config.c b/config.c
> index 0dfed68..c9fc62e 100644
> --- a/config.c
> +++ b/config.c
> @@ -14,6 +14,8 @@
> #include "string-list.h"
> #include "utf8.h"
>
> +int ignore_repo_config;
> +
> struct config_source {
> struct config_source *prev;
> union {
> @@ -1289,7 +1291,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
> int ret = 0;
> char *xdg_config = xdg_config_home("config");
> char *user_config = expand_user_path("~/.gitconfig");
> - char *repo_config = git_pathdup("config");
> + char *repo_config = ignore_repo_config ? NULL : git_pathdup("config");;
>
> current_parsing_scope = CONFIG_SCOPE_SYSTEM;
> if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index a6fdd5e..8efddaa 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -384,4 +384,13 @@ test_expect_success MINGW 'bare git dir not hidden' '
> ! is_hidden newdir
> '
>
> +test_expect_success 'init from existing directory does not confuse config' '
> + rm -rf newdir &&
> + test_config core.logallrefupdates true &&
> + git init newdir &&
> + echo true >expect &&
> + git -C newdir config --bool core.logallrefupdates >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
>
>
^ permalink raw reply
* Re: bug: 'core.logallrefupdates' is not set by default in non-bare repository
From: Jeff King @ 2016-09-02 8:47 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: doak, git@vger.kernel.org
In-Reply-To: <20160902080416.jmrctu3onfmylmeq@sigill.intra.peff.net>
On Fri, Sep 02, 2016 at 04:04:16AM -0400, Jeff King wrote:
> So here's the minimal fix that seems to work for me:
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 3a45f0b..56e7b9a 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
I also wonder if "clone" should be doing something similar. Or, for that
matter, things like git-daemon that operate outside of a repo. They work
now because they do not happen to trigger any library calls which look
at config under the hood.
Traditionally these were supposed to just use git_config_early(), but
that's really not possible when the config calls are happening behind
the scenes (e.g., when lazy-loading the config cache). And so we
eventually got rid of git_config_early() entirely.
But I wonder if we could enforce that concept automatically for config.
The simple patch below does fix this case:
diff --git a/config.c b/config.c
index 0dfed68..b62bb40 100644
--- a/config.c
+++ b/config.c
@@ -1289,7 +1289,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
int ret = 0;
char *xdg_config = xdg_config_home("config");
char *user_config = expand_user_path("~/.gitconfig");
- char *repo_config = git_pathdup("config");
+ char *repo_config = startup_info->have_repository ? git_pathdup("config") : NULL;
current_parsing_scope = CONFIG_SCOPE_SYSTEM;
if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
but it causes a few test failures. Some of those are arguably
reasonable, though. E.g., several of the diff tests use "git diff
--no-index" and expect to read local config. But "--no-index" explicitly
_avoids_ setting up the git repository, so the current code just falls
back to reading ".git/config". Which means it works when you are at the
top-level of a repository, but not in a subdir!
So I think this patch is an improvement; if we have not set up the
repository, then we should not be reading its config! (It's another
question of whether --no-index should try setup_git_directory_gently(),
but then this patch would just do the right thing).
I think "hash-object" without "-w" is in the same boat. It does not even
bother looking for a git dir, but we assume that it can see config like
core.autocrlf. It works in the top-level, but not elsewhere:
$ git init
$ git config core.autocrlf true
$ printf 'foo\r\n' >file
$ git hash-object file
257cc5642cb1a054f08cc83f2d943e56fd3ebe99
$ mkdir subdir
$ cd subdir
$ git hash-object ../file
e48b03ece74f47d1ae20075200c64aeaa01a9cdb
So there is definitely some cleanup work, but it seems like it would be
fixing a bunch of bugs.
Some of the other failures are not so obvious. In particular, t7006
tests the core.pager settings that are looked up before we set up the
git directory, and those are now broken. OTOH, I suspect that doing it
_correctly_ would fix all of the known breakages like:
not ok 46 - git -p true - core.pager overrides PAGER from subdirectory
They are hitting that same subdirectory problem mentioned above.
-Peff
^ permalink raw reply related
* [PATCH 1/2] Add a builtin helper for interactive rebases
From: Johannes Schindelin @ 2016-09-02 8:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1472805251.git.johannes.schindelin@gmx.de>
Git's interactive rebase is still implemented as a shell script, despite
its complexity. This implies that it suffers from the portability point
of view, from lack of expressibility, and of course also from
performance. The latter issue is particularly serious on Windows, where
we pay a hefty price for relying so much on POSIX.
Unfortunately, being such a huge shell script also means that we missed
the train when it would have been relatively easy to port it to C, and
instead piled feature upon feature onto that poor script that originally
never intended to be more than a slightly pimped cherry-pick in a loop.
To open the road toward better performance (in addition to all the other
benefits of C over shell scripts), let's just start *somewhere*.
The approach taken here is to add a builtin helper that at first intends
to take care of the parts of the interactive rebase that are most
affected by the performance penalties mentioned above.
In particular, after we spent all those efforts on preparing the sequencer
to process rebase -i's git-rebase-todo scripts, we implement the `git
rebase -i --continue` functionality as a new builtin, git-rebase--helper.
Once that is in place, we can work gradually on tackling the rest of the
technical debt.
Note that the rebase--helper needs to learn about the transient
--ff/--no-ff options of git-rebase, as the corresponding flag is not
persisted to, and re-read from, the state directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/rebase--helper.c | 40 ++++++++++++++++++++++++++++++++++++++++
git.c | 1 +
5 files changed, 44 insertions(+)
create mode 100644 builtin/rebase--helper.c
diff --git a/.gitignore b/.gitignore
index 05cb58a..a9b8c96 100644
--- a/.gitignore
+++ b/.gitignore
@@ -114,6 +114,7 @@
/git-read-tree
/git-rebase
/git-rebase--am
+/git-rebase--helper
/git-rebase--interactive
/git-rebase--merge
/git-receive-pack
diff --git a/Makefile b/Makefile
index d96ecb7..980e1dc 100644
--- a/Makefile
+++ b/Makefile
@@ -919,6 +919,7 @@ BUILTIN_OBJS += builtin/prune.o
BUILTIN_OBJS += builtin/pull.o
BUILTIN_OBJS += builtin/push.o
BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase--helper.o
BUILTIN_OBJS += builtin/receive-pack.o
BUILTIN_OBJS += builtin/reflog.o
BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index 6b95006..2e5de14 100644
--- a/builtin.h
+++ b/builtin.h
@@ -102,6 +102,7 @@ extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
extern int cmd_pull(int argc, const char **argv, const char *prefix);
extern int cmd_push(int argc, const char **argv, const char *prefix);
extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase__helper(int argc, const char **argv, const char *prefix);
extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
extern int cmd_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
new file mode 100644
index 0000000..ca1ebb2
--- /dev/null
+++ b/builtin/rebase--helper.c
@@ -0,0 +1,40 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "sequencer.h"
+
+static const char * const builtin_rebase_helper_usage[] = {
+ N_("git rebase--helper [<options>]"),
+ NULL
+};
+
+int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
+{
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ enum {
+ CONTINUE = 1, ABORT
+ } command = 0;
+ struct option options[] = {
+ OPT_BOOL(0, "ff", &opts.allow_ff, N_("allow fast-forward")),
+ OPT_CMDMODE(0, "continue", &command, N_("continue rebase"),
+ CONTINUE),
+ OPT_CMDMODE(0, "abort", &command, N_("abort rebase"),
+ ABORT),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+
+ opts.action = REPLAY_INTERACTIVE_REBASE;
+ opts.allow_ff = 1;
+ opts.allow_empty = 1;
+
+ argc = parse_options(argc, argv, NULL, options,
+ builtin_rebase_helper_usage, PARSE_OPT_KEEP_ARGV0);
+
+ if (command == CONTINUE && argc == 1)
+ return !!sequencer_continue(&opts);
+ if (command == ABORT && argc == 1)
+ return !!sequencer_remove_state(&opts);
+ usage_with_options(builtin_rebase_helper_usage, options);
+}
diff --git a/git.c b/git.c
index 0f1937f..26b4ad3 100644
--- a/git.c
+++ b/git.c
@@ -451,6 +451,7 @@ static struct cmd_struct commands[] = {
{ "pull", cmd_pull, RUN_SETUP | NEED_WORK_TREE },
{ "push", cmd_push, RUN_SETUP },
{ "read-tree", cmd_read_tree, RUN_SETUP },
+ { "rebase--helper", cmd_rebase__helper, RUN_SETUP | NEED_WORK_TREE },
{ "receive-pack", cmd_receive_pack },
{ "reflog", cmd_reflog, RUN_SETUP },
{ "remote", cmd_remote, RUN_SETUP },
--
2.9.3.windows.3
^ permalink raw reply related
* [PATCH 2/2] rebase -i: use the rebase--helper builtin
From: Johannes Schindelin @ 2016-09-02 8:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1472805251.git.johannes.schindelin@gmx.de>
Now that the sequencer learned to process a "normal" interactive rebase,
we use it. The original shell script is still used for "non-normal"
interactive rebases, i.e. when --root or --preserve-merges was passed.
Please note that the --root option (via the $squash_onto variable) needs
special handling only for the very first command, hence it is still okay
to use the helper upon continue/skip.
Also please note that the --no-ff setting is volatile, i.e. when the
interactive rebase is interrupted at any stage, there is no record of
it. Therefore, we have to pass it from the shell script to the
rebase--helper.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-rebase--interactive.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e558b0..022766b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1059,6 +1059,10 @@ git_rebase__interactive () {
case "$action" in
continue)
+ if test ! -d "$rewritten"
+ then
+ exec git rebase--helper ${force_rebase:+--no-ff} --continue
+ fi
# do we have anything to commit?
if git diff-index --cached --quiet HEAD --
then
@@ -1118,6 +1122,10 @@ first and then run 'git rebase --continue' again.")"
skip)
git rerere clear
+ if test ! -d "$rewritten"
+ then
+ exec git rebase--helper ${force_rebase:+--no-ff} --continue
+ fi
do_rest
return 0
;;
@@ -1307,6 +1315,11 @@ expand_todo_ids
test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
checkout_onto
+if test -z "$rebase_root" && test ! -d "$rewritten"
+then
+ require_clean_work_tree "rebase"
+ exec git rebase--helper ${force_rebase:+--no-ff} --continue
+fi
do_rest
}
--
2.9.3.windows.3
^ permalink raw reply related
* [PATCH 0/2] Let the sequencer handle the grunt work of rebase -i
From: Johannes Schindelin @ 2016-09-02 8:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
After all of these patch series y'all had to review, this is finally the
one that switches things over.
Please note that it does not (yet) handle the `git rebase -i --root`
invocation; I tried to focus on the common case, and I rarely use --root
myself.
Please note also that --preserve-merges is *not* handled.
The way I designed --preserve-merges is totally stupid and idiotic and I
do not want to spend any further time on it. You cannot "pick" merges
and hope to be able to reorder commits, for example.
And please finally note that this pair of patches does not yet yield the
full speed improvement that I promised earlier. After these patches, the
time is dominated by pre- and post-processing the todo script, at least
on Windows, so there is another patch series that ports those bits and
pieces into the rebase--helper, too.
Johannes Schindelin (2):
Add a builtin helper for interactive rebases
rebase -i: use the rebase--helper builtin
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/rebase--helper.c | 40 ++++++++++++++++++++++++++++++++++++++++
git-rebase--interactive.sh | 13 +++++++++++++
git.c | 1 +
6 files changed, 57 insertions(+)
create mode 100644 builtin/rebase--helper.c
Based-On: sequencer-i at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git sequencer-i
Published-As: https://github.com/dscho/git/releases/tag/rebase--helper-v1
Fetch-It-Via: git fetch https://github.com/dscho/git rebase--helper-v1
--
2.9.3.windows.3
base-commit: bbec81903b5e46c481fdc0cfe6f10166423526f1
^ permalink raw reply
* Re: bug: 'core.logallrefupdates' is not set by default in non-bare repository
From: Jeff King @ 2016-09-02 8:04 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: doak, git@vger.kernel.org
In-Reply-To: <1472657553.4265.63.camel@kaarsemaker.net>
On Wed, Aug 31, 2016 at 05:32:33PM +0200, Dennis Kaarsemaker wrote:
> > We may need to do something like turn off the
> > need_shared_repository_from_config in init-db, since I think it would
> > not want to ever read from the default config sources in most of its
> > code-paths (OTOH, it should in theory respect core.sharedRepository
> > in ~/.gitconfig, so maybe there is another more elegant way of
> > handling this).
>
> I would go even further and say that git init should completely ignore
> the config of a repository you happen to be in when creating a new
> repository.
Hmm. I'd think we would already be avoiding that, because we shouldn't
be calling setup_git_directory(). But some of the lazy-loaded setup is a
bit overzealous, and we blindly look at ".git/config". If we try the
same operation from a subdir of an existing repo, we _don't_ end up
confused. Eek.
So I actually wonder if that is the root of the bug. In your patch, you
disable config reading when we chdir to the new repo:
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 3a45f0b..d0fd3dc 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -493,6 +493,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> int mkdir_tried = 0;
> retry:
> if (chdir(argv[0]) < 0) {
> + /*
> + * We're creating a new repository. If we're already in another
> + * repository, ignore its config
> + */
> + ignore_repo_config = 1;
> + git_config_clear();
But I think we should go further and avoid ever looking at the original
repository in the first place. I.e., I would say that "git init" should
never ever behave differently if run in an existing repo versus outside
of one.
So really we ought to be setting ignore_repo_config from the very start
of cmd_init(), and then re-enabling it once we are "inside" the new
repo. The git_config_clear() should in theory come once we are
"inside", as well; we may have cached system/global config, and
need to flush so we read them anew along with the new local config.
OTOH, since there shouldn't be anything interesting in the new
repo-level config, I'm not sure that's really necessary. The rest of
"init" can probably proceed without caring.
I also wonder if there are other things besides config which might
accidentally read from .git (because they call git_pathdup(), and it
just blindly looks in ".git" if nobody called setup_git_directory()). So
it would be nice to have some flag for "do not ever lazy-call
setup_git_env; we do not care about any repository". But I think that's
ahrd; functions like git_pathdup() are always expected to return _some_
value, so what would they say? The best we could do is return
"/does-not-exist/" or something, but that is awfully hacky.
> @@ -500,7 +506,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> * and we know shared_repository should always be 0;
> * but just in case we play safe.
> */
> - saved = get_shared_repository();
> set_shared_repository(0);
> switch (safe_create_leading_directories_const(argv[0])) {
> case SCLD_OK:
I don't know if anybody cares about being able to set core.sharedRepository
from ~/.gitconfig. It didn't work until v2.9.0 anyway (when I moved it
out of the repository-format check), but it seems like you _should_ be
able to set it and have it Just Work.
And in that case, this "we know shared_repository should always be 0" is
not true, and we would want to keep doing the save/set-to-0/restore
dance here.
> @@ -524,6 +528,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> } else if (0 < argc) {
> usage(init_db_usage[0]);
> }
> +
> + need_shared_repository_from_config = 1;
> + ignore_repo_config = 0;
> + git_config_clear();
This is the part I think we could actually skip. The only thing we might
not have loaded is the "config" we just wrote to the new repository. But
I don't think we have to care about what is in it.
> diff --git a/config.c b/config.c
> index 0dfed68..2df0189 100644
> --- a/config.c
> +++ b/config.c
> @@ -1304,7 +1304,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
> ret += git_config_from_file(fn, user_config, data);
>
> current_parsing_scope = CONFIG_SCOPE_REPO;
> - if (repo_config && !access_or_die(repo_config, R_OK, 0))
> + if (repo_config && !ignore_repo_config && !access_or_die(repo_config, R_OK, 0))
> ret += git_config_from_file(fn, repo_config, data);
We probably want to intercept the call to git_pathdup() earlier than
this, if the point is not to touch any of the lazy-load setup_git_dir()
stuff at all. The effect is the same for config, but I think it makes
sense to have as little effect as possible.
So here's the minimal fix that seems to work for me:
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 3a45f0b..56e7b9a 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -484,6 +484,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ ignore_repo_config = 1;
+
argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
if (real_git_dir && !is_absolute_path(real_git_dir))
diff --git a/cache.h b/cache.h
index b780a91..13b78e4 100644
--- a/cache.h
+++ b/cache.h
@@ -1582,6 +1582,13 @@ enum config_origin_type {
CONFIG_ORIGIN_CMDLINE
};
+/*
+ * If non-zero, git_config() will avoid any attempt to find the repo config;
+ * this is useful for programs like git-init that might look at config before
+ * actually setting up the new repository.
+ */
+extern int ignore_repo_config;
+
typedef int (*config_fn_t)(const char *, const char *, void *);
extern int git_default_config(const char *, const char *, void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
diff --git a/config.c b/config.c
index 0dfed68..c9fc62e 100644
--- a/config.c
+++ b/config.c
@@ -14,6 +14,8 @@
#include "string-list.h"
#include "utf8.h"
+int ignore_repo_config;
+
struct config_source {
struct config_source *prev;
union {
@@ -1289,7 +1291,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
int ret = 0;
char *xdg_config = xdg_config_home("config");
char *user_config = expand_user_path("~/.gitconfig");
- char *repo_config = git_pathdup("config");
+ char *repo_config = ignore_repo_config ? NULL : git_pathdup("config");;
current_parsing_scope = CONFIG_SCOPE_SYSTEM;
if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index a6fdd5e..8efddaa 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -384,4 +384,13 @@ test_expect_success MINGW 'bare git dir not hidden' '
! is_hidden newdir
'
+test_expect_success 'init from existing directory does not confuse config' '
+ rm -rf newdir &&
+ test_config core.logallrefupdates true &&
+ git init newdir &&
+ echo true >expect &&
+ git -C newdir config --bool core.logallrefupdates >actual &&
+ test_cmp expect actual
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2016-09-02 7:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Jeff King, Git
In-Reply-To: <xmqqzinrteql.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 1 Sep 2016, Junio C Hamano wrote:
> Hopefully that [patch removing the -<pid> suffix] would help making
> Dscho's "what are the failed tests?" logic simpler.
Of course.
It also makes sure that those 2 hours I spent on writing and perfecting
the sed magic were spent in vain... ;-)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 27/34] sequencer (rebase -i): differentiate between comments and 'noop'
From: Johannes Schindelin @ 2016-09-02 7:32 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git, Junio C Hamano
In-Reply-To: <1472746523.4680.30.camel@kaarsemaker.net>
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]
Hi Dennis,
On Thu, 1 Sep 2016, Dennis Kaarsemaker wrote:
> /*
> * Note that ordering matters in this enum. Not only must it match the
> * mapping below, it is also divided into several sections that matter.
> * When adding new commands, make sure you add it in the right section.
> */
> enum todo_command {
> /* All commands that handle commits */
> TODO_PICK,
> ...
> /* All commands that do something else than pick */
> TODO_EXEC,
> ...
> /* All commands that do nothing but are counted for reporting progress */
> TODO_NOOP,
> ...
> /* Comments, which are not counted
> TODO_COMMENT
> }
I like it! Changed accordingly.
Thanks!
Dscho
^ permalink raw reply
* Re: [PATCH 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-09-02 7:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dennis Kaarsemaker, git
In-Reply-To: <xmqqr393wkof.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 1 Sep 2016, Junio C Hamano wrote:
> For those who were not paying attention on the 'master' front during
> this pre-release period [*1*], I have to point out that the scripted
> Porcelain has been updated to lose the Anglo-centric st/nd/rd/th and
> this series would want to get updated to match.
>
>
> [Footnote]
>
> *1* Why weren't you? Repent! ;-)
I tried to. But, you know, I was kinda busy with a couple of patch series.
In any case, I changed the code this morning. Can't say that I like those
forced last-minute changes.
Ciao,
Johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox