* [PATCH v2 0/2] Show HTTP headers of failed requests with GIT_CURL_VERBOSE
From: Masaya Suzuki @ 2018-12-29 19:44 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20181228014720.206443-1-masayasuzuki@google.com>
When GIT_CURL_VERBOSE is set, libcurl produces request/response headers
to stderr. However, if the response is an error response and
CURLOPT_FAILONERROR is set, libcurl stops parsing the response, and it
won't dump the headers. Showing HTTP response headers is useful for
debugging, especially for non-OK responses.
To this end, the caller of libcurl needs to handle HTTP request failures by
themselves. The first patch makes git prepared to handle those failures. The
second patch actually unsets CURLOPT_FAILONERROR.
Masaya Suzuki (2):
Change how HTTP response body is returned
Unset CURLOPT_FAILONERROR
http.c | 103 +++++++++++++++++++----------------
http.h | 1 -
remote-curl.c | 30 ++++++++--
t/lib-httpd/apache.conf | 1 +
t/t5581-http-curl-verbose.sh | 28 ++++++++++
5 files changed, 110 insertions(+), 53 deletions(-)
create mode 100755 t/t5581-http-curl-verbose.sh
--
2.20.1.415.g653613c723-goog
^ permalink raw reply
* Re: [PATCH/RFC v1 1/1] Support working-tree-encoding "UTF-16LE-BOM"
From: Philip Oakley @ 2018-12-29 17:54 UTC (permalink / raw)
To: Adrián Gimeno Balaguer, tboegi, git; +Cc: brian m. carlson
In-Reply-To: <CADN+U_Mo4Ui-rmZe1+xoHOMA4koXGNpJ5XEGYoYZfYPGqP9VPQ@mail.gmail.com>
(adding Brian as cc who was in the original thread)
On 29/12/2018 15:48, Adrián Gimeno Balaguer wrote:
> Hello again.
>
> I appreciate the grown interest in this issue.
>
> Torsten, may I know what is the benefit on your code? My PR solved it
> by only tweaking the utf8.c's function 'has_prohibited_utf_bom', which
> is likely the shortest way:
>
> https://github.com/git/git/pull/550/files
My main complaint with the PR would be the lack of documentation updates.
As the discussion has highlighted, whatever our solution, we will need
to tell the users in plain and simple terms which parts of which
standards are being used, and why we need to be somehow 'different'.
That is because a revision control system must be able to recover the
original, for use in the original software tool, not just interpret it
is some alternate form. The standards generally abdicate responsibility
for the last step ;-)
I did not fully understand the conversion process you proposed, as I
assumed(?) that on receipt of the source file, the Git conversion to
utf-8 would convert the 16-bit BOM to the three byte utf-8 BOM byte
sequence `EF BB BF` which has lost any knowledge of the original BE/LE
coding.
Or, are we saying that the the 16-bit BOM is being interpreted as, a)
the BE/LE indicator and b) a genuine "ZERO WIDTH NON-BREAKING SPACE"
which is stored as the two byte utf-8 character code, again loosing
(once stored as a blob object) the BE/LE indication.
Or, we see the BOM, note the endianness and then loose the BOM character
when converting to utf-8. My ignorance of this step is starting to show.
Regular users are probably even more confused, hence my hope for some
documentation.
Given the above confusions, and many more when exploring the internet,
the provision of a new, extra, clear, name for the encoding, as
suggested by Torsten does offer an advantage in that it explicitly
(rather than implicitly) makes plain what we are trying to do, without
squeezing it in 'under the radar'.
That said, assuming an appropriate internal utf-8 Git coding that does
remember the BE/LE state [if so how?] then the PR is a neat trick.
Torsten's patch also suffers from the lack of user facing documentation.
>
> In order to make sure everything is clear, here is a case list of
> current Git behaviour and new one after my PR, regarding this issue.
>
> Current behaviour:
>
> - Placing 'test.txt working-tree-encoding=UTF-16' for a new test.txt
> file with either UTF-16 BE or LE BOM, and comitting everything -> The
> file gets re-encoded from UTF-8 (as stored internally), to UTF-16 and
> the default system/libiconv endianness -> Problem (as long as user
> required the opposite endianness for any reason on his project). As a
> note, user can see however human-readable diffs on that file.
>
> - Placing 'test.txt working-tree-encoding=UTF-16LE' or 'test.txt
> working-tree-encoding=UTF-16BE' for a new test.txt file with either
> UTF-16 BE or LE BOM, and comitting everything: we assume user is doing
> this because he requires that exact endianness, thus he writes it in
> order to attempt preserving it -> Git prohibites commiting it, also no
> human-readable diff is shown in the diff viewer/tool being used, but
> file is simply shown as binary.
>
> New behaviour:
>
> - Just got too lazy to repeat it all over, read my PR description:
> https://github.com/git/git/pull/550
"In this PR: Git only prohibites the opposite BOM than the one in
working-tree-encoding (e.g. if declared LE, then it denies BE BOM
presence within the associated file, of the declared UTF-16/UTF-32).
This way the user can now make Git operations which were previously
impossible, with the only requisite being to match the endianness of
working-tree-encoding attribute with the associated file/s."
>
> - Git translations may need to be tweaked to in order to be consistent
> with new behaviour.
>
> Thanks for your attention.
>
--
Philip
^ permalink raw reply
* [PATCH v4 15/16] sequencer: use edit_todo_list() in complete_action()
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This changes complete_action() to use edit_todo_list(), now that it can
handle the initial edit of the todo list.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Changes due to conflicts with nd/the-index.
sequencer.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0500a32e80..127bb0b68e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4703,6 +4703,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
struct todo_list new_todo = TODO_LIST_INIT;
struct strbuf *buf = &todo_list->buf;
struct object_id oid;
+ int res;
get_oid(onto, &oid);
shortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV);
@@ -4727,24 +4728,16 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return error(_("nothing to do"));
}
- if (todo_list_write_to_file(r, todo_list, todo_file,
- shortrevisions, shortonto, -1,
- flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP))
- return error_errno(_("could not write '%s'"), todo_file);
-
- if (copy_file(rebase_path_todo_backup(), todo_file, 0666))
- return error(_("could not copy '%s' to '%s'."), todo_file,
- rebase_path_todo_backup());
-
- if (launch_sequence_editor(todo_file, &new_todo.buf, NULL)) {
+ res = edit_todo_list(r, todo_list, &new_todo, shortrevisions,
+ shortonto, flags);
+ if (res == -1)
+ return -1;
+ else if (res == -2) {
apply_autostash(opts);
sequencer_remove_state(opts);
return -1;
- }
-
- strbuf_stripspace(&new_todo.buf, 1);
- if (new_todo.buf.len == 0) {
+ } else if (res == -3) {
apply_autostash(opts);
sequencer_remove_state(opts);
todo_list_release(&new_todo);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 14/16] rebase-interactive: rewrite edit_todo_list() to handle the initial edit
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
edit_todo_list() is changed to work on a todo_list, and to handle the
initial edition of the todo list (ie. making a backup of the todo
list).
It does not check for dropped commits yet, as todo_list_check() does not
take the commits that have already been processed by the rebase (ie. the
todo list is edited in the middle of a rebase session).
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Changes due to conflicts with nd/the-index.
builtin/rebase--interactive.c | 24 +++++++++++++++++-
rebase-interactive.c | 48 ++++++++++++++++++-----------------
rebase-interactive.h | 4 ++-
sequencer.c | 3 +--
sequencer.h | 1 +
5 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 2dbf8fc08b..645ac587f7 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -13,6 +13,28 @@ static GIT_PATH_FUNC(path_state_dir, "rebase-merge/")
static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto")
static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive")
+static int edit_todo_file(unsigned flags)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT,
+ new_todo = TODO_LIST_INIT;
+
+ if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ strbuf_stripspace(&todo_list.buf, 1);
+ if (!edit_todo_list(the_repository, &todo_list,
+ &new_todo, NULL, NULL, flags) &&
+ todo_list_write_to_file(the_repository, &new_todo, todo_file, NULL, NULL,
+ -1, flags & ~(TODO_LIST_SHORTEN_IDS)) < 0)
+ return error_errno(_("could not write '%s'"), todo_file);
+
+ todo_list_release(&todo_list);
+ todo_list_release(&new_todo);
+
+ return 0;
+}
+
static int get_revision_ranges(const char *upstream, const char *onto,
const char **head_hash,
char **revisions, char **shortrevisions)
@@ -242,7 +264,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
break;
}
case EDIT_TODO:
- ret = edit_todo_list(the_repository, flags);
+ ret = edit_todo_file(flags);
break;
case SHOW_CURRENT_PATCH: {
struct child_process cmd = CHILD_PROCESS_INIT;
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 32f95002df..7542e70a55 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -87,35 +87,37 @@ void append_todo_help(unsigned keep_empty, int command_count,
}
}
-int edit_todo_list(struct repository *r, unsigned flags)
+int edit_todo_list(struct repository *r, struct todo_list *todo_list,
+ struct todo_list *new_todo, const char *shortrevisions,
+ const char *shortonto, unsigned flags)
{
const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- int res = 0;
-
- if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- strbuf_stripspace(&todo_list.buf, 1);
- todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
- if (todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1,
- flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP)) {
- todo_list_release(&todo_list);
- return -1;
+ unsigned initial = shortrevisions && shortonto;
+
+ if (initial) {
+ todo_list_write_to_file(r, todo_list, todo_file, shortrevisions, shortonto,
+ -1, flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP);
+
+ if (copy_file(rebase_path_todo_backup(), todo_file, 0666))
+ return error(_("could not copy '%s' to '%s'."), todo_file,
+ rebase_path_todo_backup());
+ } else {
+ todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list);
+ todo_list_write_to_file(r, todo_list, todo_file, NULL, NULL, -1,
+ flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP);
}
- strbuf_reset(&todo_list.buf);
- if (launch_sequence_editor(todo_file, &todo_list.buf, NULL)) {
- todo_list_release(&todo_list);
- return -1;
- }
+ if (launch_sequence_editor(todo_file, &new_todo->buf, NULL))
+ return -2;
- if (!todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list))
- res = todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1,
- flags & ~(TODO_LIST_SHORTEN_IDS));
+ strbuf_stripspace(&new_todo->buf, 1);
+ if (initial && new_todo->buf.len == 0)
+ return -3;
- todo_list_release(&todo_list);
- return res;
+ if (!initial)
+ todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo);
+
+ return 0;
}
define_commit_slab(commit_seen, unsigned char);
diff --git a/rebase-interactive.h b/rebase-interactive.h
index 2f6675eabd..42cc3f865d 100644
--- a/rebase-interactive.h
+++ b/rebase-interactive.h
@@ -7,7 +7,9 @@ struct repository;
void append_todo_help(unsigned keep_empty, int command_count,
const char *shortrevisions, const char *shortonto,
struct strbuf *buf);
-int edit_todo_list(struct repository *r, unsigned flags);
+int edit_todo_list(struct repository *r, struct todo_list *todo_list,
+ struct todo_list *new_todo, const char *shortrevisions,
+ const char *shortonto, unsigned flags);
int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
#endif
diff --git a/sequencer.c b/sequencer.c
index dfc705291d..0500a32e80 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -55,8 +55,7 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
* file and written to the tail of 'done'.
*/
GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
-static GIT_PATH_FUNC(rebase_path_todo_backup,
- "rebase-merge/git-rebase-todo.backup")
+GIT_PATH_FUNC(rebase_path_todo_backup, "rebase-merge/git-rebase-todo.backup")
/*
* The rebase command lines that have already been processed. A line
diff --git a/sequencer.h b/sequencer.h
index dcbd682e63..7dd85bb399 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -10,6 +10,7 @@ struct repository;
const char *git_path_commit_editmsg(void);
const char *git_path_seq_dir(void);
const char *rebase_path_todo(void);
+const char *rebase_path_todo_backup(void);
#define APPEND_SIGNOFF_DEDUP (1u << 0)
--
2.20.1
^ permalink raw reply related
* [PATCH v4 16/16] rebase--interactive: move transform_todo_file() to rebase--interactive.c
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
As transform_todo_file() is only needed inside of rebase--interactive.c,
it is moved there from sequencer.c.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
builtin/rebase--interactive.c | 26 +++++++++++++++++++++++++-
sequencer.c | 23 -----------------------
sequencer.h | 1 -
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 645ac587f7..7f1e88a087 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -35,6 +35,30 @@ static int edit_todo_file(unsigned flags)
return 0;
}
+static int transform_todo_file(unsigned flags)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res;
+
+ if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf,
+ &todo_list)) {
+ todo_list_release(&todo_list);
+ return error(_("unusable todo list: '%s'"), todo_file);
+ }
+
+ res = todo_list_write_to_file(the_repository, &todo_list, todo_file,
+ NULL, NULL, -1, flags);
+ todo_list_release(&todo_list);
+
+ if (res)
+ return error_errno(_("could not write '%s'."), todo_file);
+ return 0;
+}
+
static int get_revision_ranges(const char *upstream, const char *onto,
const char **head_hash,
char **revisions, char **shortrevisions)
@@ -277,7 +301,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
}
case SHORTEN_OIDS:
case EXPAND_OIDS:
- ret = transform_todo_file(the_repository, flags);
+ ret = transform_todo_file(flags);
break;
case CHECK_TODO_LIST:
ret = check_todo_list_from_file(the_repository);
diff --git a/sequencer.c b/sequencer.c
index 127bb0b68e..6566247a7c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4587,29 +4587,6 @@ int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
return res;
}
-int transform_todo_file(struct repository *r, unsigned flags)
-{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- int res;
-
- if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
- todo_list_release(&todo_list);
- return error(_("unusable todo list: '%s'"), todo_file);
- }
-
- res = todo_list_write_to_file(r, &todo_list, todo_file,
- NULL, NULL, -1, flags);
- todo_list_release(&todo_list);
-
- if (res)
- return error_errno(_("could not write '%s'."), todo_file);
- return 0;
-}
-
static const char edit_todo_list_advice[] =
N_("You can fix this with 'git rebase --edit-todo' "
"and then run 'git rebase --continue'.\n"
diff --git a/sequencer.h b/sequencer.h
index 7dd85bb399..bb3ca5783b 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -146,7 +146,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
int sequencer_add_exec_commands(struct repository *r,
struct string_list *commands);
-int transform_todo_file(struct repository *r, unsigned flags);
int check_todo_list_from_file(struct repository *r);
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 13/16] rebase-interactive: append_todo_help() changes
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This moves the writing of the comment "Rebase $shortrevisions onto
$shortonto ($command_count commands)" from todo_list_write_to_file() to
append_todo_help().
shortrevisions, shortonto, and command_count are passed as parameters to
append_todo_help().
During the initial edit of the todo list, shortrevisions and shortonto
are not NULL. Therefore, if shortrevisions or shortonto is NULL, then
edit_todo would be true, otherwise it would be false. Thus, edit_todo
is removed from the parameters of append_todo_help().
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Slight rewording of the message and changes due to conflicts with
nd/the-index.
rebase-interactive.c | 12 +++++++++++-
rebase-interactive.h | 3 ++-
sequencer.c | 17 ++++-------------
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 994f0f9753..32f95002df 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -28,7 +28,8 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)
return MISSING_COMMIT_CHECK_IGNORE;
}
-void append_todo_help(unsigned edit_todo, unsigned keep_empty,
+void append_todo_help(unsigned keep_empty, int command_count,
+ const char *shortrevisions, const char *shortonto,
struct strbuf *buf)
{
const char *msg = _("\nCommands:\n"
@@ -48,6 +49,15 @@ void append_todo_help(unsigned edit_todo, unsigned keep_empty,
". specified). Use -c <commit> to reword the commit message.\n"
"\n"
"These lines can be re-ordered; they are executed from top to bottom.\n");
+ unsigned edit_todo = !(shortrevisions && shortonto);
+
+ if (!edit_todo) {
+ strbuf_addch(buf, '\n');
+ strbuf_commented_addf(buf, Q_("Rebase %s onto %s (%d command)",
+ "Rebase %s onto %s (%d commands)",
+ command_count),
+ shortrevisions, shortonto, command_count);
+ }
strbuf_add_commented_lines(buf, msg, strlen(msg));
diff --git a/rebase-interactive.h b/rebase-interactive.h
index dd0d717bc1..2f6675eabd 100644
--- a/rebase-interactive.h
+++ b/rebase-interactive.h
@@ -4,7 +4,8 @@
struct strbuf;
struct repository;
-void append_todo_help(unsigned edit_todo, unsigned keep_empty,
+void append_todo_help(unsigned keep_empty, int command_count,
+ const char *shortrevisions, const char *shortonto,
struct strbuf *buf);
int edit_todo_list(struct repository *r, unsigned flags);
int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
diff --git a/sequencer.c b/sequencer.c
index dfdba5cec0..dfc705291d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4574,22 +4574,13 @@ int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
const char *file, const char *shortrevisions,
const char *shortonto, int num, unsigned flags)
{
- int edit_todo = !(shortrevisions && shortonto), res;
+ int res;
struct strbuf buf = STRBUF_INIT;
todo_list_to_strbuf(r, todo_list, &buf, num, flags);
-
- if (flags & TODO_LIST_APPEND_TODO_HELP) {
- int command_count = count_commands(todo_list);
- if (!edit_todo) {
- strbuf_addch(&buf, '\n');
- strbuf_commented_addf(&buf, Q_("Rebase %s onto %s (%d command)",
- "Rebase %s onto %s (%d commands)",
- command_count),
- shortrevisions, shortonto, command_count);
- }
- append_todo_help(edit_todo, flags & TODO_LIST_KEEP_EMPTY, &buf);
- }
+ if (flags & TODO_LIST_APPEND_TODO_HELP)
+ append_todo_help(flags & TODO_LIST_KEEP_EMPTY, count_commands(todo_list),
+ shortrevisions, shortonto, &buf);
res = write_message(buf.buf, buf.len, file, 0);
strbuf_release(&buf);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 06/16] sequencer: refactor check_todo_list() to work on a todo_list
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This refactors check_todo_list() to work on a todo_list to avoid
redundant reads and writes to the disk. The function is renamed
todo_list_check(). The parsing of the two todo lists is left to the
caller.
As rebase -p still need to check the todo list from the disk, a new
function is introduced, check_todo_list_from_file(). It reads the file
from the disk, parses it, pass the todo_list to todo_list_check(), and
writes it back to the disk.
As get_missing_commit_check_level() and the enum
missing_commit_check_level are no longer needed inside of sequencer.c,
they are moved to rebase-interactive.c, and made static again.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
builtin/rebase--interactive.c | 2 +-
rebase-interactive.c | 91 ++++++++++++++++++++++++-
rebase-interactive.h | 1 +
sequencer.c | 121 +++++++---------------------------
sequencer.h | 9 +--
5 files changed, 116 insertions(+), 108 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 0898eb4c59..df19ccaeb9 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -256,7 +256,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
ret = transform_todo_file(the_repository, flags);
break;
case CHECK_TODO_LIST:
- ret = check_todo_list(the_repository);
+ ret = check_todo_list_from_file(the_repository);
break;
case REARRANGE_SQUASH:
ret = rearrange_squash(the_repository);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 842fa07e7e..6157247e1f 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -1,8 +1,32 @@
#include "cache.h"
#include "commit.h"
-#include "rebase-interactive.h"
#include "sequencer.h"
+#include "rebase-interactive.h"
#include "strbuf.h"
+#include "commit-slab.h"
+#include "config.h"
+
+enum missing_commit_check_level {
+ MISSING_COMMIT_CHECK_IGNORE = 0,
+ MISSING_COMMIT_CHECK_WARN,
+ MISSING_COMMIT_CHECK_ERROR
+};
+
+static enum missing_commit_check_level get_missing_commit_check_level(void)
+{
+ const char *value;
+
+ if (git_config_get_value("rebase.missingcommitscheck", &value) ||
+ !strcasecmp("ignore", value))
+ return MISSING_COMMIT_CHECK_IGNORE;
+ if (!strcasecmp("warn", value))
+ return MISSING_COMMIT_CHECK_WARN;
+ if (!strcasecmp("error", value))
+ return MISSING_COMMIT_CHECK_ERROR;
+ warning(_("unrecognized setting %s for option "
+ "rebase.missingCommitsCheck. Ignoring."), value);
+ return MISSING_COMMIT_CHECK_IGNORE;
+}
void append_todo_help(unsigned edit_todo, unsigned keep_empty,
struct strbuf *buf)
@@ -89,3 +113,68 @@ int edit_todo_list(struct repository *r, unsigned flags)
return 0;
}
+
+define_commit_slab(commit_seen, unsigned char);
+/*
+ * Check if the user dropped some commits by mistake
+ * Behaviour determined by rebase.missingCommitsCheck.
+ * Check if there is an unrecognized command or a
+ * bad SHA-1 in a command.
+ */
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
+{
+ enum missing_commit_check_level check_level = get_missing_commit_check_level();
+ struct strbuf missing = STRBUF_INIT;
+ int res = 0, i;
+ struct commit_seen commit_seen;
+
+ init_commit_seen(&commit_seen);
+
+ if (check_level == MISSING_COMMIT_CHECK_IGNORE)
+ goto leave_check;
+
+ /* Mark the commits in git-rebase-todo as seen */
+ for (i = 0; i < new_todo->nr; i++) {
+ struct commit *commit = new_todo->items[i].commit;
+ if (commit)
+ *commit_seen_at(&commit_seen, commit) = 1;
+ }
+
+ /* Find commits in git-rebase-todo.backup yet unseen */
+ for (i = old_todo->nr - 1; i >= 0; i--) {
+ struct todo_item *item = old_todo->items + i;
+ struct commit *commit = item->commit;
+ if (commit && !*commit_seen_at(&commit_seen, commit)) {
+ strbuf_addf(&missing, " - %s %.*s\n",
+ find_unique_abbrev(&commit->object.oid, DEFAULT_ABBREV),
+ item->arg_len,
+ old_todo->buf.buf + item->arg_offset);
+ *commit_seen_at(&commit_seen, commit) = 1;
+ }
+ }
+
+ /* Warn about missing commits */
+ if (!missing.len)
+ goto leave_check;
+
+ if (check_level == MISSING_COMMIT_CHECK_ERROR)
+ res = 1;
+
+ fprintf(stderr,
+ _("Warning: some commits may have been dropped accidentally.\n"
+ "Dropped commits (newer to older):\n"));
+
+ /* Make the list user-friendly and display */
+ fputs(missing.buf, stderr);
+ strbuf_release(&missing);
+
+ fprintf(stderr, _("To avoid this message, use \"drop\" to "
+ "explicitly remove a commit.\n\n"
+ "Use 'git config rebase.missingCommitsCheck' to change "
+ "the level of warnings.\n"
+ "The possible behaviours are: ignore, warn, error.\n\n"));
+
+leave_check:
+ clear_commit_seen(&commit_seen);
+ return res;
+}
diff --git a/rebase-interactive.h b/rebase-interactive.h
index 17b6c9f6d0..dd0d717bc1 100644
--- a/rebase-interactive.h
+++ b/rebase-interactive.h
@@ -7,5 +7,6 @@ struct repository;
void append_todo_help(unsigned edit_todo, unsigned keep_empty,
struct strbuf *buf);
int edit_todo_list(struct repository *r, unsigned flags);
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo);
#endif
diff --git a/sequencer.c b/sequencer.c
index b932ca34f2..347a1a701f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4599,112 +4599,37 @@ int transform_todo_file(struct repository *r, unsigned flags)
return 0;
}
-enum missing_commit_check_level get_missing_commit_check_level(void)
-{
- const char *value;
-
- if (git_config_get_value("rebase.missingcommitscheck", &value) ||
- !strcasecmp("ignore", value))
- return MISSING_COMMIT_CHECK_IGNORE;
- if (!strcasecmp("warn", value))
- return MISSING_COMMIT_CHECK_WARN;
- if (!strcasecmp("error", value))
- return MISSING_COMMIT_CHECK_ERROR;
- warning(_("unrecognized setting %s for option "
- "rebase.missingCommitsCheck. Ignoring."), value);
- return MISSING_COMMIT_CHECK_IGNORE;
-}
+static const char edit_todo_list_advice[] =
+N_("You can fix this with 'git rebase --edit-todo' "
+"and then run 'git rebase --continue'.\n"
+"Or you can abort the rebase with 'git rebase"
+" --abort'.\n");
-define_commit_slab(commit_seen, unsigned char);
-/*
- * Check if the user dropped some commits by mistake
- * Behaviour determined by rebase.missingCommitsCheck.
- * Check if there is an unrecognized command or a
- * bad SHA-1 in a command.
- */
-int check_todo_list(struct repository *r)
+int check_todo_list_from_file(struct repository *r)
{
- enum missing_commit_check_level check_level = get_missing_commit_check_level();
- struct strbuf todo_file = STRBUF_INIT;
- struct todo_list todo_list = TODO_LIST_INIT;
- struct strbuf missing = STRBUF_INIT;
- int advise_to_edit_todo = 0, res = 0, i;
- struct commit_seen commit_seen;
-
- init_commit_seen(&commit_seen);
+ struct todo_list old_todo = TODO_LIST_INIT, new_todo = TODO_LIST_INIT;
+ int res = 0;
- strbuf_addstr(&todo_file, rebase_path_todo());
- if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
+ if (strbuf_read_file_or_whine(&new_todo.buf, rebase_path_todo()) < 0) {
res = -1;
- goto leave_check;
- }
- advise_to_edit_todo = res =
- todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
-
- if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
- goto leave_check;
-
- /* Mark the commits in git-rebase-todo as seen */
- for (i = 0; i < todo_list.nr; i++) {
- struct commit *commit = todo_list.items[i].commit;
- if (commit)
- *commit_seen_at(&commit_seen, commit) = 1;
+ goto out;
}
- todo_list_release(&todo_list);
- strbuf_addstr(&todo_file, ".backup");
- if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
+ if (strbuf_read_file_or_whine(&old_todo.buf, rebase_path_todo_backup()) < 0) {
res = -1;
- goto leave_check;
- }
- strbuf_release(&todo_file);
- res = !!todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
-
- /* Find commits in git-rebase-todo.backup yet unseen */
- for (i = todo_list.nr - 1; i >= 0; i--) {
- struct todo_item *item = todo_list.items + i;
- struct commit *commit = item->commit;
- if (commit && !*commit_seen_at(&commit_seen, commit)) {
- strbuf_addf(&missing, " - %s %.*s\n",
- short_commit_name(commit),
- item->arg_len,
- todo_list.buf.buf + item->arg_offset);
- *commit_seen_at(&commit_seen, commit) = 1;
- }
+ goto out;
}
- /* Warn about missing commits */
- if (!missing.len)
- goto leave_check;
-
- if (check_level == MISSING_COMMIT_CHECK_ERROR)
- advise_to_edit_todo = res = 1;
-
- fprintf(stderr,
- _("Warning: some commits may have been dropped accidentally.\n"
- "Dropped commits (newer to older):\n"));
-
- /* Make the list user-friendly and display */
- fputs(missing.buf, stderr);
- strbuf_release(&missing);
-
- fprintf(stderr, _("To avoid this message, use \"drop\" to "
- "explicitly remove a commit.\n\n"
- "Use 'git config rebase.missingCommitsCheck' to change "
- "the level of warnings.\n"
- "The possible behaviours are: ignore, warn, error.\n\n"));
-
-leave_check:
- clear_commit_seen(&commit_seen);
- strbuf_release(&todo_file);
- todo_list_release(&todo_list);
-
- if (advise_to_edit_todo)
- fprintf(stderr,
- _("You can fix this with 'git rebase --edit-todo' "
- "and then run 'git rebase --continue'.\n"
- "Or you can abort the rebase with 'git rebase"
- " --abort'.\n"));
+ res = todo_list_parse_insn_buffer(r, old_todo.buf.buf, &old_todo);
+ if (!res)
+ res = todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo);
+ if (!res)
+ res = todo_list_check(&old_todo, &new_todo);
+ if (res)
+ fprintf(stderr, _(edit_todo_list_advice));
+out:
+ todo_list_release(&old_todo);
+ todo_list_release(&new_todo);
return res;
}
@@ -4882,7 +4807,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
todo_list_release(&todo_list);
- if (check_todo_list(r)) {
+ if (check_todo_list_from_file(r)) {
checkout_onto(opts, onto_name, onto, orig_head);
return -1;
}
diff --git a/sequencer.h b/sequencer.h
index 4643fb3b69..07a8418b0d 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -64,12 +64,6 @@ struct replay_opts {
};
#define REPLAY_OPTS_INIT { .action = -1, .current_fixups = STRBUF_INIT }
-enum missing_commit_check_level {
- MISSING_COMMIT_CHECK_IGNORE = 0,
- MISSING_COMMIT_CHECK_WARN,
- MISSING_COMMIT_CHECK_ERROR
-};
-
int write_message(const void *buf, size_t len, const char *filename,
int append_eol);
@@ -152,8 +146,7 @@ int sequencer_make_script(struct repository *r, FILE *out, int argc,
int sequencer_add_exec_commands(struct repository *r, const char *command);
int transform_todo_file(struct repository *r, unsigned flags);
-enum missing_commit_check_level get_missing_commit_check_level(void);
-int check_todo_list(struct repository *r);
+int check_todo_list_from_file(struct repository *r);
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
const char *onto, const char *orig_head, const char *cmd,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 11/16] sequencer: refactor skip_unnecessary_picks() to work on a todo_list
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This refactors skip_unnecessary_picks() to work on a todo_list. As this
function is only called by complete_action() (and thus is not used by
rebase -p), the file-handling logic is completely dropped here.
Instead of truncating the todo list’s buffer, the items are moved to
the beginning of the list, eliminating the need to reparse the list.
This also means its buffer cannot be directly written to the disk.
rewrite_file() is then removed, as it is now unused.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
sequencer.c | 78 ++++++++++++-----------------------------------------
1 file changed, 17 insertions(+), 61 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ea5cea81a8..dfdba5cec0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4655,52 +4655,22 @@ int check_todo_list_from_file(struct repository *r)
return res;
}
-static int rewrite_file(const char *path, const char *buf, size_t len)
-{
- int rc = 0;
- int fd = open(path, O_WRONLY | O_TRUNC);
- if (fd < 0)
- return error_errno(_("could not open '%s' for writing"), path);
- if (write_in_full(fd, buf, len) < 0)
- rc = error_errno(_("could not write to '%s'"), path);
- if (close(fd) && !rc)
- rc = error_errno(_("could not close '%s'"), path);
- return rc;
-}
-
/* skip picking commits whose parents are unchanged */
-static int skip_unnecessary_picks(struct repository *r, struct object_id *output_oid)
+static int skip_unnecessary_picks(struct repository *r,
+ struct todo_list *todo_list,
+ struct object_id *output_oid)
{
- const char *todo_file = rebase_path_todo();
- struct strbuf buf = STRBUF_INIT;
- struct todo_list todo_list = TODO_LIST_INIT;
struct object_id *parent_oid;
- int fd, i;
-
- if (!read_oneliner(&buf, rebase_path_onto(), 0))
- return error(_("could not read 'onto'"));
- if (get_oid(buf.buf, output_oid)) {
- strbuf_release(&buf);
- return error(_("need a HEAD to fixup"));
- }
- strbuf_release(&buf);
-
- if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
- return -1;
- if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
- todo_list_release(&todo_list);
- return -1;
- }
+ int i;
- for (i = 0; i < todo_list.nr; i++) {
- struct todo_item *item = todo_list.items + i;
+ for (i = 0; i < todo_list->nr; i++) {
+ struct todo_item *item = todo_list->items + i;
if (item->command >= TODO_NOOP)
continue;
if (item->command != TODO_PICK)
break;
if (parse_commit(item->commit)) {
- todo_list_release(&todo_list);
return error(_("could not parse commit '%s'"),
oid_to_hex(&item->commit->object.oid));
}
@@ -4714,37 +4684,21 @@ static int skip_unnecessary_picks(struct repository *r, struct object_id *output
oidcpy(output_oid, &item->commit->object.oid);
}
if (i > 0) {
- int offset = get_item_line_offset(&todo_list, i);
const char *done_path = rebase_path_done();
- fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
- if (fd < 0) {
- error_errno(_("could not open '%s' for writing"),
- done_path);
- todo_list_release(&todo_list);
- return -1;
- }
- if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
+ if (todo_list_write_to_file(r, todo_list, done_path, NULL, NULL, i, 0)) {
error_errno(_("could not write to '%s'"), done_path);
- todo_list_release(&todo_list);
- close(fd);
return -1;
}
- close(fd);
- if (rewrite_file(rebase_path_todo(), todo_list.buf.buf + offset,
- todo_list.buf.len - offset) < 0) {
- todo_list_release(&todo_list);
- return -1;
- }
+ MOVE_ARRAY(todo_list->items, todo_list->items + i, todo_list->nr - i);
+ todo_list->nr -= i;
+ todo_list->current = 0;
- todo_list.current = i;
- if (is_fixup(peek_command(&todo_list, 0)))
- record_in_rewritten(output_oid, peek_command(&todo_list, 0));
+ if (is_fixup(peek_command(todo_list, 0)))
+ record_in_rewritten(output_oid, peek_command(todo_list, 0));
}
- todo_list_release(&todo_list);
-
return 0;
}
@@ -4817,6 +4771,11 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return -1;
}
+ if (opts->allow_ff && skip_unnecessary_picks(r, &new_todo, &oid)) {
+ todo_list_release(&new_todo);
+ return error(_("could not skip unnecessary pick commands"));
+ }
+
if (todo_list_write_to_file(r, &new_todo, todo_file, NULL, NULL, -1,
flags & ~(TODO_LIST_SHORTEN_IDS))) {
todo_list_release(&new_todo);
@@ -4825,9 +4784,6 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
todo_list_release(&new_todo);
- if (opts->allow_ff && skip_unnecessary_picks(r, &oid))
- return error(_("could not skip unnecessary pick commands"));
-
if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head))
return -1;
--
2.20.1
^ permalink raw reply related
* [PATCH v4 12/16] rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
Just like complete_action(), edit_todo_list() used a
function (transform_todo_file()) that read the todo list from the disk
and wrote it back, resulting in useless disk accesses.
This changes edit_todo_list() to call directly todo_list_write_to_file()
instead.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Changes due to conflicts with nd/the-index.
rebase-interactive.c | 38 ++++++++++++++++----------------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 6157247e1f..994f0f9753 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -79,39 +79,33 @@ void append_todo_help(unsigned edit_todo, unsigned keep_empty,
int edit_todo_list(struct repository *r, unsigned flags)
{
- struct strbuf buf = STRBUF_INIT;
const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res = 0;
- if (strbuf_read_file(&buf, todo_file, 0) < 0)
+ if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
- strbuf_stripspace(&buf, 1);
- if (write_message(buf.buf, buf.len, todo_file, 0)) {
- strbuf_release(&buf);
+ strbuf_stripspace(&todo_list.buf, 1);
+ todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
+ if (todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1,
+ flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP)) {
+ todo_list_release(&todo_list);
return -1;
}
- strbuf_release(&buf);
-
- transform_todo_file(r, flags | TODO_LIST_SHORTEN_IDS);
-
- if (strbuf_read_file(&buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- append_todo_help(1, 0, &buf);
- if (write_message(buf.buf, buf.len, todo_file, 0)) {
- strbuf_release(&buf);
+ strbuf_reset(&todo_list.buf);
+ if (launch_sequence_editor(todo_file, &todo_list.buf, NULL)) {
+ todo_list_release(&todo_list);
return -1;
}
- strbuf_release(&buf);
+ if (!todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list))
+ res = todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1,
+ flags & ~(TODO_LIST_SHORTEN_IDS));
- if (launch_sequence_editor(todo_file, NULL, NULL))
- return -1;
-
- transform_todo_file(r, flags & ~(TODO_LIST_SHORTEN_IDS));
-
- return 0;
+ todo_list_release(&todo_list);
+ return res;
}
define_commit_slab(commit_seen, unsigned char);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 10/16] sequencer: change complete_action() to use the refactored functions
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
complete_action() used functions that read the todo-list file, made some
changes to it, and wrote it back to the disk.
The previous commits were dedicated to separate the part that deals with
the file from the actual logic of these functions. Now that this is
done, we can call directly the "logic" functions to avoid useless file
access.
The parsing of the list has to be done by the caller. If the buffer of
the todo list provided by the caller is empty, a `noop' command is
directly added to the todo list, without touching the buffer.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
builtin/rebase--interactive.c | 20 +++------
sequencer.c | 81 +++++++++++++++--------------------
sequencer.h | 2 +-
3 files changed, 42 insertions(+), 61 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 92026739c9..2dbf8fc08b 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -71,7 +71,6 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
const char *head_hash = NULL;
char *revisions = NULL, *shortrevisions = NULL;
struct argv_array make_script_args = ARGV_ARRAY_INIT;
- FILE *todo_list_file;
struct todo_list todo_list = TODO_LIST_INIT;
if (prepare_branch_to_be_rebased(opts, switch_to))
@@ -94,14 +93,6 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
if (!upstream && squash_onto)
write_file(path_squash_onto(), "%s\n", squash_onto);
- todo_list_file = fopen(rebase_path_todo(), "w");
- if (!todo_list_file) {
- free(revisions);
- free(shortrevisions);
-
- return error_errno(_("could not open %s"), rebase_path_todo());
- }
-
argv_array_pushl(&make_script_args, "", revisions, NULL);
if (restrict_revision)
argv_array_push(&make_script_args, restrict_revision);
@@ -109,16 +100,17 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
ret = sequencer_make_script(the_repository, &todo_list.buf,
make_script_args.argc, make_script_args.argv,
flags);
- fputs(todo_list.buf.buf, todo_list_file);
- fclose(todo_list_file);
if (ret)
error(_("could not generate todo list"));
else {
discard_cache();
- ret = complete_action(the_repository, opts, flags,
- shortrevisions, onto_name, onto,
- head_hash, commands, autosquash);
+ if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf,
+ &todo_list))
+ BUG("unusable todo list");
+
+ ret = complete_action(the_repository, opts, flags, shortrevisions, onto_name,
+ onto, head_hash, commands, autosquash, &todo_list);
}
free(revisions);
diff --git a/sequencer.c b/sequencer.c
index f6527fb418..ea5cea81a8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4748,100 +4748,89 @@ static int skip_unnecessary_picks(struct repository *r, struct object_id *output
return 0;
}
+static int todo_list_rearrange_squash(struct todo_list *todo_list);
+
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
const char *onto, const char *orig_head, struct string_list *commands,
- unsigned autosquash)
+ unsigned autosquash, struct todo_list *todo_list)
{
const char *shortonto, *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- struct strbuf *buf = &(todo_list.buf);
+ struct todo_list new_todo = TODO_LIST_INIT;
+ struct strbuf *buf = &todo_list->buf;
struct object_id oid;
- struct stat st;
get_oid(onto, &oid);
shortonto = find_unique_abbrev(&oid, DEFAULT_ABBREV);
- if (!lstat(todo_file, &st) && st.st_size == 0 &&
- write_message("noop\n", 5, todo_file, 0))
- return -1;
+ if (buf->len == 0) {
+ struct todo_item *item = append_new_todo(todo_list);
+ item->command = TODO_NOOP;
+ item->commit = NULL;
+ item->arg_len = item->arg_offset = item->flags = item->offset_in_buf = 0;
+ }
- if (autosquash && rearrange_squash_in_todo_file(r))
+ if (autosquash && todo_list_rearrange_squash(todo_list))
return -1;
if (commands->nr)
- sequencer_add_exec_commands(r, commands);
+ todo_list_add_exec_commands(todo_list, commands);
- if (strbuf_read_file(buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- if (todo_list_parse_insn_buffer(r, buf->buf, &todo_list)) {
- todo_list_release(&todo_list);
- return error(_("unusable todo list: '%s'"), todo_file);
- }
-
- if (count_commands(&todo_list) == 0) {
+ if (count_commands(todo_list) == 0) {
apply_autostash(opts);
sequencer_remove_state(opts);
- todo_list_release(&todo_list);
return error(_("nothing to do"));
}
- strbuf_addch(buf, '\n');
- strbuf_commented_addf(buf, Q_("Rebase %s onto %s (%d command)",
- "Rebase %s onto %s (%d commands)",
- count_commands(&todo_list)),
- shortrevisions, shortonto, count_commands(&todo_list));
- append_todo_help(0, flags & TODO_LIST_KEEP_EMPTY, buf);
-
- if (write_message(buf->buf, buf->len, todo_file, 0)) {
- todo_list_release(&todo_list);
- return -1;
- }
+ if (todo_list_write_to_file(r, todo_list, todo_file,
+ shortrevisions, shortonto, -1,
+ flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP))
+ return error_errno(_("could not write '%s'"), todo_file);
if (copy_file(rebase_path_todo_backup(), todo_file, 0666))
return error(_("could not copy '%s' to '%s'."), todo_file,
rebase_path_todo_backup());
- if (transform_todo_file(r, flags | TODO_LIST_SHORTEN_IDS))
- return error(_("could not transform the todo list"));
-
- strbuf_reset(buf);
-
- if (launch_sequence_editor(todo_file, buf, NULL)) {
+ if (launch_sequence_editor(todo_file, &new_todo.buf, NULL)) {
apply_autostash(opts);
sequencer_remove_state(opts);
- todo_list_release(&todo_list);
return -1;
}
- strbuf_stripspace(buf, 1);
- if (buf->len == 0) {
+ strbuf_stripspace(&new_todo.buf, 1);
+ if (new_todo.buf.len == 0) {
apply_autostash(opts);
sequencer_remove_state(opts);
- todo_list_release(&todo_list);
+ todo_list_release(&new_todo);
return error(_("nothing to do"));
}
- todo_list_release(&todo_list);
-
- if (check_todo_list_from_file(r)) {
+ if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) ||
+ todo_list_check(todo_list, &new_todo)) {
+ fprintf(stderr, _(edit_todo_list_advice));
checkout_onto(opts, onto_name, onto, orig_head);
+ todo_list_release(&new_todo);
+
return -1;
}
- if (transform_todo_file(r, flags & ~(TODO_LIST_SHORTEN_IDS)))
- return error(_("could not transform the todo list"));
+ if (todo_list_write_to_file(r, &new_todo, todo_file, NULL, NULL, -1,
+ flags & ~(TODO_LIST_SHORTEN_IDS))) {
+ todo_list_release(&new_todo);
+ return error_errno(_("could not write '%s'"), todo_file);
+ }
+
+ todo_list_release(&new_todo);
if (opts->allow_ff && skip_unnecessary_picks(r, &oid))
return error(_("could not skip unnecessary pick commands"));
if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head))
return -1;
-;
+
if (require_clean_work_tree(r, "rebase", "", 1, 1))
return -1;
diff --git a/sequencer.h b/sequencer.h
index d75049c15f..dcbd682e63 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -150,7 +150,7 @@ int check_todo_list_from_file(struct repository *r);
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
const char *onto, const char *orig_head, struct string_list *commands,
- unsigned autosquash);
+ unsigned autosquash, struct todo_list *todo_list);
int rearrange_squash_in_todo_file(struct repository *r);
extern const char sign_off_header[];
--
2.20.1
^ permalink raw reply related
* [PATCH v4 09/16] sequencer: make sequencer_make_script() write its script to a strbuf
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This makes sequencer_make_script() write its script to a strbuf (ie. the
buffer of a todo_list) instead of a FILE. This reduce the amount of
read/write made by rebase interactive.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Changes due to conflicts with nd/the-index.
builtin/rebase--interactive.c | 13 ++++++-----
sequencer.c | 41 +++++++++++++++--------------------
sequencer.h | 5 ++---
3 files changed, 28 insertions(+), 31 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 4f0eae9239..92026739c9 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -71,7 +71,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
const char *head_hash = NULL;
char *revisions = NULL, *shortrevisions = NULL;
struct argv_array make_script_args = ARGV_ARRAY_INIT;
- FILE *todo_list;
+ FILE *todo_list_file;
+ struct todo_list todo_list = TODO_LIST_INIT;
if (prepare_branch_to_be_rebased(opts, switch_to))
return -1;
@@ -93,8 +94,8 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
if (!upstream && squash_onto)
write_file(path_squash_onto(), "%s\n", squash_onto);
- todo_list = fopen(rebase_path_todo(), "w");
- if (!todo_list) {
+ todo_list_file = fopen(rebase_path_todo(), "w");
+ if (!todo_list_file) {
free(revisions);
free(shortrevisions);
@@ -105,10 +106,11 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
if (restrict_revision)
argv_array_push(&make_script_args, restrict_revision);
- ret = sequencer_make_script(the_repository, todo_list,
+ ret = sequencer_make_script(the_repository, &todo_list.buf,
make_script_args.argc, make_script_args.argv,
flags);
- fclose(todo_list);
+ fputs(todo_list.buf.buf, todo_list_file);
+ fclose(todo_list_file);
if (ret)
error(_("could not generate todo list"));
@@ -121,6 +123,7 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
free(revisions);
free(shortrevisions);
+ todo_list_release(&todo_list);
argv_array_clear(&make_script_args);
return ret;
diff --git a/sequencer.c b/sequencer.c
index 931ad9848e..f6527fb418 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4154,7 +4154,7 @@ static const char *label_oid(struct object_id *oid, const char *label,
}
static int make_script_with_merges(struct pretty_print_context *pp,
- struct rev_info *revs, FILE *out,
+ struct rev_info *revs, struct strbuf *out,
unsigned flags)
{
int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
@@ -4299,7 +4299,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
* gathering commits not yet shown, reversing the list on the fly,
* then outputting that list (labeling revisions as needed).
*/
- fprintf(out, "%s onto\n", cmd_label);
+ strbuf_addf(out, "%s onto\n", cmd_label);
for (iter = tips; iter; iter = iter->next) {
struct commit_list *list = NULL, *iter2;
@@ -4309,9 +4309,9 @@ static int make_script_with_merges(struct pretty_print_context *pp,
entry = oidmap_get(&state.commit2label, &commit->object.oid);
if (entry)
- fprintf(out, "\n%c Branch %s\n", comment_line_char, entry->string);
+ strbuf_addf(out, "\n%c Branch %s\n", comment_line_char, entry->string);
else
- fprintf(out, "\n");
+ strbuf_addch(out, '\n');
while (oidset_contains(&interesting, &commit->object.oid) &&
!oidset_contains(&shown, &commit->object.oid)) {
@@ -4324,8 +4324,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,
}
if (!commit)
- fprintf(out, "%s %s\n", cmd_reset,
- rebase_cousins ? "onto" : "[new root]");
+ strbuf_addf(out, "%s %s\n", cmd_reset,
+ rebase_cousins ? "onto" : "[new root]");
else {
const char *to = NULL;
@@ -4338,12 +4338,12 @@ static int make_script_with_merges(struct pretty_print_context *pp,
&state);
if (!to || !strcmp(to, "onto"))
- fprintf(out, "%s onto\n", cmd_reset);
+ strbuf_addf(out, "%s onto\n", cmd_reset);
else {
strbuf_reset(&oneline);
pretty_print_commit(pp, commit, &oneline);
- fprintf(out, "%s %s # %s\n",
- cmd_reset, to, oneline.buf);
+ strbuf_addf(out, "%s %s # %s\n",
+ cmd_reset, to, oneline.buf);
}
}
@@ -4352,11 +4352,11 @@ static int make_script_with_merges(struct pretty_print_context *pp,
entry = oidmap_get(&commit2todo, oid);
/* only show if not already upstream */
if (entry)
- fprintf(out, "%s\n", entry->string);
+ strbuf_addf(out, "%s\n", entry->string);
entry = oidmap_get(&state.commit2label, oid);
if (entry)
- fprintf(out, "%s %s\n",
- cmd_label, entry->string);
+ strbuf_addf(out, "%s %s\n",
+ cmd_label, entry->string);
oidset_insert(&shown, oid);
}
@@ -4378,13 +4378,11 @@ static int make_script_with_merges(struct pretty_print_context *pp,
return 0;
}
-int sequencer_make_script(struct repository *r, FILE *out,
- int argc, const char **argv,
- unsigned flags)
+int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
+ const char **argv, unsigned flags)
{
char *format = NULL;
struct pretty_print_context pp = {0};
- struct strbuf buf = STRBUF_INIT;
struct rev_info revs;
struct commit *commit;
int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
@@ -4427,16 +4425,13 @@ int sequencer_make_script(struct repository *r, FILE *out,
if (!is_empty && (commit->object.flags & PATCHSAME))
continue;
- strbuf_reset(&buf);
if (!keep_empty && is_empty)
- strbuf_addf(&buf, "%c ", comment_line_char);
- strbuf_addf(&buf, "%s %s ", insn,
+ strbuf_addf(out, "%c ", comment_line_char);
+ strbuf_addf(out, "%s %s ", insn,
oid_to_hex(&commit->object.oid));
- pretty_print_commit(&pp, commit, &buf);
- strbuf_addch(&buf, '\n');
- fputs(buf.buf, out);
+ pretty_print_commit(&pp, commit, out);
+ strbuf_addch(out, '\n');
}
- strbuf_release(&buf);
return 0;
}
diff --git a/sequencer.h b/sequencer.h
index e680fe2daa..d75049c15f 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -140,9 +140,8 @@ int sequencer_remove_state(struct replay_opts *opts);
#define TODO_LIST_REBASE_COUSINS (1U << 4)
#define TODO_LIST_APPEND_TODO_HELP (1U << 5)
-int sequencer_make_script(struct repository *r, FILE *out, int argc,
- const char **argv,
- unsigned flags);
+int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
+ const char **argv, unsigned flags);
int sequencer_add_exec_commands(struct repository *r,
struct string_list *commands);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 08/16] sequencer: refactor rearrange_squash() to work on a todo_list
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This refactors rearrange_squash() to work on a todo_list to avoid
redundant reads and writes. The function is renamed
todo_list_rearrange_squash().
The old version created a new buffer, which was directly written to the
disk. This new version creates a new item list by just copying items
from the old item list, without creating a new buffer. This eliminates
the need to reparse the todo list, but this also means its buffer cannot
be directly written to the disk.
As rebase -p still need to check the todo list from the disk, a new
function is introduced, rearrange_squash_in_todo_file().
complete_action() still uses rearrange_squash_in_todo_file() for now.
This will be changed in a future commit.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Slight rewording of the commit message and changes due to conflicts with
nd/the-index.
builtin/rebase--interactive.c | 2 +-
sequencer.c | 92 ++++++++++++++++++-----------------
sequencer.h | 2 +-
3 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 53056ee713..4f0eae9239 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -266,7 +266,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
ret = check_todo_list_from_file(the_repository);
break;
case REARRANGE_SQUASH:
- ret = rearrange_squash(the_repository);
+ ret = rearrange_squash_in_todo_file(the_repository);
break;
case ADD_EXEC:
ret = sequencer_add_exec_commands(the_repository, &commands);
diff --git a/sequencer.c b/sequencer.c
index 2df64b3677..931ad9848e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4771,7 +4771,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
write_message("noop\n", 5, todo_file, 0))
return -1;
- if (autosquash && rearrange_squash(r))
+ if (autosquash && rearrange_squash_in_todo_file(r))
return -1;
if (commands->nr)
@@ -4877,21 +4877,13 @@ define_commit_slab(commit_todo_item, struct todo_item *);
* message will have to be retrieved from the commit (as the oneline in the
* script cannot be trusted) in order to normalize the autosquash arrangement.
*/
-int rearrange_squash(struct repository *r)
+static int todo_list_rearrange_squash(struct todo_list *todo_list)
{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
struct hashmap subject2item;
- int res = 0, rearranged = 0, *next, *tail, i;
+ int rearranged = 0, *next, *tail, i, nr = 0, alloc = 0;
char **subjects;
struct commit_todo_item commit_todo;
-
- if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
- return -1;
- if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
- todo_list_release(&todo_list);
- return -1;
- }
+ struct todo_item *items = NULL;
init_commit_todo_item(&commit_todo);
/*
@@ -4904,13 +4896,13 @@ int rearrange_squash(struct repository *r)
* be moved to appear after the i'th.
*/
hashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,
- NULL, todo_list.nr);
- ALLOC_ARRAY(next, todo_list.nr);
- ALLOC_ARRAY(tail, todo_list.nr);
- ALLOC_ARRAY(subjects, todo_list.nr);
- for (i = 0; i < todo_list.nr; i++) {
+ NULL, todo_list->nr);
+ ALLOC_ARRAY(next, todo_list->nr);
+ ALLOC_ARRAY(tail, todo_list->nr);
+ ALLOC_ARRAY(subjects, todo_list->nr);
+ for (i = 0; i < todo_list->nr; i++) {
struct strbuf buf = STRBUF_INIT;
- struct todo_item *item = todo_list.items + i;
+ struct todo_item *item = todo_list->items + i;
const char *commit_buffer, *subject, *p;
size_t subject_len;
int i2 = -1;
@@ -4923,7 +4915,6 @@ int rearrange_squash(struct repository *r)
}
if (is_fixup(item->command)) {
- todo_list_release(&todo_list);
clear_commit_todo_item(&commit_todo);
return error(_("the script was already rearranged."));
}
@@ -4958,7 +4949,7 @@ int rearrange_squash(struct repository *r)
*commit_todo_item_at(&commit_todo, commit2))
/* found by commit name */
i2 = *commit_todo_item_at(&commit_todo, commit2)
- - todo_list.items;
+ - todo_list->items;
else {
/* copy can be a prefix of the commit subject */
for (i2 = 0; i2 < i; i2++)
@@ -4971,7 +4962,7 @@ int rearrange_squash(struct repository *r)
}
if (i2 >= 0) {
rearranged = 1;
- todo_list.items[i].command =
+ todo_list->items[i].command =
starts_with(subject, "fixup!") ?
TODO_FIXUP : TODO_SQUASH;
if (next[i2] < 0)
@@ -4989,10 +4980,8 @@ int rearrange_squash(struct repository *r)
}
if (rearranged) {
- struct strbuf buf = STRBUF_INIT;
-
- for (i = 0; i < todo_list.nr; i++) {
- enum todo_command command = todo_list.items[i].command;
+ for (i = 0; i < todo_list->nr; i++) {
+ enum todo_command command = todo_list->items[i].command;
int cur = i;
/*
@@ -5003,37 +4992,50 @@ int rearrange_squash(struct repository *r)
continue;
while (cur >= 0) {
- const char *bol =
- get_item_line(&todo_list, cur);
- const char *eol =
- get_item_line(&todo_list, cur + 1);
-
- /* replace 'pick', by 'fixup' or 'squash' */
- command = todo_list.items[cur].command;
- if (is_fixup(command)) {
- strbuf_addstr(&buf,
- todo_command_info[command].str);
- bol += strcspn(bol, " \t");
- }
-
- strbuf_add(&buf, bol, eol - bol);
-
+ ALLOC_GROW(items, nr + 1, alloc);
+ items[nr++] = todo_list->items[cur];
cur = next[cur];
}
}
- res = rewrite_file(todo_file, buf.buf, buf.len);
- strbuf_release(&buf);
+ FREE_AND_NULL(todo_list->items);
+ todo_list->items = items;
+ todo_list->nr = nr;
+ todo_list->alloc = alloc;
}
free(next);
free(tail);
- for (i = 0; i < todo_list.nr; i++)
+ for (i = 0; i < todo_list->nr; i++)
free(subjects[i]);
free(subjects);
hashmap_free(&subject2item, 1);
- todo_list_release(&todo_list);
clear_commit_todo_item(&commit_todo);
- return res;
+
+ return 0;
+}
+
+int rearrange_squash_in_todo_file(struct repository *r)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res = 0;
+
+ if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
+ return -1;
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
+ todo_list_release(&todo_list);
+ return -1;
+ }
+
+ res = todo_list_rearrange_squash(&todo_list);
+ if (!res)
+ res = todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1, 0);
+
+ todo_list_release(&todo_list);
+
+ if (res)
+ return error_errno(_("could not write '%s'."), todo_file);
+ return 0;
}
diff --git a/sequencer.h b/sequencer.h
index 1d9d66b0cd..e680fe2daa 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -152,7 +152,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
const char *shortrevisions, const char *onto_name,
const char *onto, const char *orig_head, struct string_list *commands,
unsigned autosquash);
-int rearrange_squash(struct repository *r);
+int rearrange_squash_in_todo_file(struct repository *r);
extern const char sign_off_header[];
--
2.20.1
^ permalink raw reply related
* [PATCH v4 07/16] sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This refactors sequencer_add_exec_commands() to work on a todo_list to
avoid redundant reads and writes to the disk.
Instead of inserting the `exec' commands between the other commands and
re-parsing the buffer at the end, they are appended to the buffer once,
and a new list of items is created. Items from the old list are copied
across and new `exec' items are appended when necessary. This
eliminates the need to reparse the buffer, but this also means we have
to use todo_list_write_to_disk() to write the file.
todo_list_add_exec_commands() and sequencer_add_exec_commands() are
modified to take a string list instead of a string -- one item for each
command. This makes it easier to insert a new command to the todo list
for each command to execute.
sequencer_add_exec_commands() still reads the todo list from the disk,
as it is needed by rebase -p.
complete_action() still uses sequencer_add_exec_commands() for now.
This will be changed in a future commit.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
builtin/rebase--interactive.c | 15 +++--
sequencer.c | 110 +++++++++++++++++++++-------------
sequencer.h | 5 +-
3 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index df19ccaeb9..53056ee713 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -65,7 +65,7 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
const char *onto, const char *onto_name,
const char *squash_onto, const char *head_name,
const char *restrict_revision, char *raw_strategies,
- const char *cmd, unsigned autosquash)
+ struct string_list *commands, unsigned autosquash)
{
int ret;
const char *head_hash = NULL;
@@ -116,7 +116,7 @@ static int do_interactive_rebase(struct replay_opts *opts, unsigned flags,
discard_cache();
ret = complete_action(the_repository, opts, flags,
shortrevisions, onto_name, onto,
- head_hash, cmd, autosquash);
+ head_hash, commands, autosquash);
}
free(revisions);
@@ -139,6 +139,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL,
*squash_onto = NULL, *upstream = NULL, *head_name = NULL,
*switch_to = NULL, *cmd = NULL;
+ struct string_list commands = STRING_LIST_INIT_DUP;
char *raw_strategies = NULL;
enum {
NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH,
@@ -221,6 +222,12 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
warning(_("--[no-]rebase-cousins has no effect without "
"--rebase-merges"));
+ if (cmd && *cmd) {
+ string_list_split(&commands, cmd, '\n', -1);
+ if (strlen(commands.items[commands.nr - 1].string) == 0)
+ --commands.nr;
+ }
+
switch (command) {
case NONE:
if (!onto && !upstream)
@@ -228,7 +235,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
ret = do_interactive_rebase(&opts, flags, switch_to, upstream, onto,
onto_name, squash_onto, head_name, restrict_revision,
- raw_strategies, cmd, autosquash);
+ raw_strategies, &commands, autosquash);
break;
case SKIP: {
struct string_list merge_rr = STRING_LIST_INIT_DUP;
@@ -262,7 +269,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
ret = rearrange_squash(the_repository);
break;
case ADD_EXEC:
- ret = sequencer_add_exec_commands(the_repository, cmd);
+ ret = sequencer_add_exec_commands(the_repository, &commands);
break;
default:
BUG("invalid command '%d'", command);
diff --git a/sequencer.c b/sequencer.c
index 347a1a701f..2df64b3677 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4440,25 +4440,27 @@ int sequencer_make_script(struct repository *r, FILE *out,
return 0;
}
-/*
- * Add commands after pick and (series of) squash/fixup commands
- * in the todo list.
- */
-int sequencer_add_exec_commands(struct repository *r,
- const char *commands)
+static void todo_list_add_exec_commands(struct todo_list *todo_list,
+ struct string_list *commands)
{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- struct strbuf *buf = &todo_list.buf;
- size_t offset = 0, commands_len = strlen(commands);
- int i, insert;
+ struct strbuf *buf = &todo_list->buf;
+ size_t base_offset = buf->len;
+ int i, insert, nr = 0, alloc = 0;
+ struct todo_item *items = NULL, *base_items = NULL;
- if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
- return error(_("could not read '%s'."), todo_file);
+ base_items = xcalloc(commands->nr, sizeof(struct todo_item));
+ for (i = 0; i < commands->nr; ++i) {
+ size_t command_len = strlen(commands->items[i].string);
- if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
- todo_list_release(&todo_list);
- return error(_("unusable todo list: '%s'"), todo_file);
+ strbuf_addstr(buf, commands->items[i].string);
+ strbuf_addch(buf, '\n');
+
+ base_items[i].command = TODO_EXEC;
+ base_items[i].offset_in_buf = base_offset;
+ base_items[i].arg_offset = base_offset + strlen("exec ");
+ base_items[i].arg_len = command_len - strlen("exec ");
+
+ base_offset += command_len + 1;
}
/*
@@ -4467,38 +4469,62 @@ int sequencer_add_exec_commands(struct repository *r,
* those chains if there are any.
*/
insert = -1;
- for (i = 0; i < todo_list.nr; i++) {
- enum todo_command command = todo_list.items[i].command;
-
- if (insert >= 0) {
- /* skip fixup/squash chains */
- if (command == TODO_COMMENT)
- continue;
- else if (is_fixup(command)) {
- insert = i + 1;
- continue;
- }
- strbuf_insert(buf,
- todo_list.items[insert].offset_in_buf +
- offset, commands, commands_len);
- offset += commands_len;
+ for (i = 0; i < todo_list->nr; i++) {
+ enum todo_command command = todo_list->items[i].command;
+ if (insert >= 0 && command != TODO_COMMENT && !is_fixup(command)) {
+ ALLOC_GROW(items, nr + commands->nr, alloc);
+ COPY_ARRAY(items + nr, base_items, commands->nr);
+ nr += commands->nr;
insert = -1;
}
- if (command == TODO_PICK || command == TODO_MERGE)
+ ALLOC_GROW(items, nr + 1, alloc);
+ items[nr++] = todo_list->items[i];
+
+ if (command == TODO_PICK || command == TODO_MERGE || is_fixup(command))
insert = i + 1;
}
/* insert or append final <commands> */
- if (insert >= 0 && insert < todo_list.nr)
- strbuf_insert(buf, todo_list.items[insert].offset_in_buf +
- offset, commands, commands_len);
- else if (insert >= 0 || !offset)
- strbuf_add(buf, commands, commands_len);
+ if (insert >= 0 || nr == todo_list->nr) {
+ ALLOC_GROW(items, nr + commands->nr, alloc);
+ COPY_ARRAY(items + nr, base_items, commands->nr);
+ nr += commands->nr;
+ }
+
+ free(base_items);
+ FREE_AND_NULL(todo_list->items);
+ todo_list->items = items;
+ todo_list->nr = nr;
+ todo_list->alloc = alloc;
+}
- i = write_message(buf->buf, buf->len, todo_file, 0);
+/*
+ * Add commands after pick and (series of) squash/fixup commands
+ * in the todo list.
+ */
+int sequencer_add_exec_commands(struct repository *r,
+ struct string_list *commands)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res;
+
+ if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
+ todo_list_release(&todo_list);
+ return error(_("unusable todo list: '%s'"), todo_file);
+ }
+
+ todo_list_add_exec_commands(&todo_list, commands);
+ res = todo_list_write_to_file(r, &todo_list, todo_file, NULL, NULL, -1, 0);
todo_list_release(&todo_list);
- return i;
+
+ if (res)
+ return error_errno(_("could not write '%s'."), todo_file);
+ return 0;
}
static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,
@@ -4729,7 +4755,7 @@ static int skip_unnecessary_picks(struct repository *r, struct object_id *output
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
- const char *onto, const char *orig_head, const char *cmd,
+ const char *onto, const char *orig_head, struct string_list *commands,
unsigned autosquash)
{
const char *shortonto, *todo_file = rebase_path_todo();
@@ -4748,8 +4774,8 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
if (autosquash && rearrange_squash(r))
return -1;
- if (cmd && *cmd)
- sequencer_add_exec_commands(r, cmd);
+ if (commands->nr)
+ sequencer_add_exec_commands(r, commands);
if (strbuf_read_file(buf, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
diff --git a/sequencer.h b/sequencer.h
index 07a8418b0d..1d9d66b0cd 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -144,12 +144,13 @@ int sequencer_make_script(struct repository *r, FILE *out, int argc,
const char **argv,
unsigned flags);
-int sequencer_add_exec_commands(struct repository *r, const char *command);
+int sequencer_add_exec_commands(struct repository *r,
+ struct string_list *commands);
int transform_todo_file(struct repository *r, unsigned flags);
int check_todo_list_from_file(struct repository *r);
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
- const char *onto, const char *orig_head, const char *cmd,
+ const char *onto, const char *orig_head, struct string_list *commands,
unsigned autosquash);
int rearrange_squash(struct repository *r);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 05/16] sequencer: introduce todo_list_write_to_file()
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This introduces a new function to recreate the text of a todo list from
its commands and write it to a file. This will be useful as the next
few commits will change the use of the buffer in struct todo_list so it
will no longer be a mirror of the file on disk.
This functionality already exists in todo_list_transform(), but this
function was made to replace the buffer of a todo list, which is not
what we want here. Thus, the part of todo_list_transform() that
replaces the buffer is dropped, and the function is renamed
todo_list_to_strbuf(). It is called by todo_list_write_to_file() to
fill the buffer to write to the disk.
todo_list_write_to_file() can also take care of appending the help text
to the buffer before writing it to the disk, or to write only the first
n items of the list. This feature will be used by
skip_unnecessary_picks(), which has to write done commands in a file.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
sequencer.c | 61 +++++++++++++++++++++++++++++++++++------------------
sequencer.h | 11 ++++++----
2 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 6b4eb7a9ba..b932ca34f2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4501,26 +4501,28 @@ int sequencer_add_exec_commands(struct repository *r,
return i;
}
-void todo_list_transform(struct repository *r, struct todo_list *todo_list,
- unsigned flags)
+static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,
+ struct strbuf *buf, int num, unsigned flags)
{
- struct strbuf buf = STRBUF_INIT;
struct todo_item *item;
- int i;
+ int i, max = todo_list->nr;
- for (item = todo_list->items, i = 0; i < todo_list->nr; i++, item++) {
+ if (num > 0 && num < max)
+ max = num;
+
+ for (item = todo_list->items, i = 0; i < max; i++, item++) {
/* if the item is not a command write it and continue */
if (item->command >= TODO_COMMENT) {
- strbuf_addf(&buf, "%.*s\n", item->arg_len,
+ strbuf_addf(buf, "%.*s\n", item->arg_len,
todo_list->buf.buf + item->arg_offset);
continue;
}
/* add command to the buffer */
if (flags & TODO_LIST_ABBREVIATE_CMDS)
- strbuf_addch(&buf, command_to_char(item->command));
+ strbuf_addch(buf, command_to_char(item->command));
else
- strbuf_addstr(&buf, command_to_string(item->command));
+ strbuf_addstr(buf, command_to_string(item->command));
/* add commit id */
if (item->commit) {
@@ -4530,28 +4532,48 @@ void todo_list_transform(struct repository *r, struct todo_list *todo_list,
if (item->command == TODO_MERGE) {
if (item->flags & TODO_EDIT_MERGE_MSG)
- strbuf_addstr(&buf, " -c");
+ strbuf_addstr(buf, " -c");
else
- strbuf_addstr(&buf, " -C");
+ strbuf_addstr(buf, " -C");
}
- strbuf_addf(&buf, " %s", oid);
+ strbuf_addf(buf, " %s", oid);
}
/* add all the rest */
if (!item->arg_len)
- strbuf_addch(&buf, '\n');
+ strbuf_addch(buf, '\n');
else
- strbuf_addf(&buf, " %.*s\n", item->arg_len,
+ strbuf_addf(buf, " %.*s\n", item->arg_len,
todo_list->buf.buf + item->arg_offset);
}
+}
- strbuf_reset(&todo_list->buf);
- strbuf_add(&todo_list->buf, buf.buf, buf.len);
+int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
+ const char *file, const char *shortrevisions,
+ const char *shortonto, int num, unsigned flags)
+{
+ int edit_todo = !(shortrevisions && shortonto), res;
+ struct strbuf buf = STRBUF_INIT;
+
+ todo_list_to_strbuf(r, todo_list, &buf, num, flags);
+
+ if (flags & TODO_LIST_APPEND_TODO_HELP) {
+ int command_count = count_commands(todo_list);
+ if (!edit_todo) {
+ strbuf_addch(&buf, '\n');
+ strbuf_commented_addf(&buf, Q_("Rebase %s onto %s (%d command)",
+ "Rebase %s onto %s (%d commands)",
+ command_count),
+ shortrevisions, shortonto, command_count);
+ }
+ append_todo_help(edit_todo, flags & TODO_LIST_KEEP_EMPTY, &buf);
+ }
+
+ res = write_message(buf.buf, buf.len, file, 0);
strbuf_release(&buf);
- if (todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list))
- BUG("unusable todo list");
+ return res;
}
int transform_todo_file(struct repository *r, unsigned flags)
@@ -4568,9 +4590,8 @@ int transform_todo_file(struct repository *r, unsigned flags)
return error(_("unusable todo list: '%s'"), todo_file);
}
- todo_list_transform(r, &todo_list, flags);
-
- res = write_message(todo_list.buf.buf, todo_list.buf.len, todo_file, 0);
+ res = todo_list_write_to_file(r, &todo_list, todo_file,
+ NULL, NULL, -1, flags);
todo_list_release(&todo_list);
if (res)
diff --git a/sequencer.h b/sequencer.h
index 53ffe7371c..4643fb3b69 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -121,8 +121,9 @@ struct todo_list {
int todo_list_parse_insn_buffer(struct repository *r, char *buf,
struct todo_list *todo_list);
-void todo_list_transform(struct repository *r, struct todo_list *todo_list,
- unsigned flags);
+int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
+ const char *file, const char *shortrevisions,
+ const char *shortonto, int num, unsigned flags);
void todo_list_release(struct todo_list *todo_list);
/* Call this to setup defaults before parsing command line options */
@@ -143,8 +144,10 @@ int sequencer_remove_state(struct replay_opts *opts);
* commits should be rebased onto the new base, this flag needs to be passed.
*/
#define TODO_LIST_REBASE_COUSINS (1U << 4)
-int sequencer_make_script(struct repository *repo, FILE *out,
- int argc, const char **argv,
+#define TODO_LIST_APPEND_TODO_HELP (1U << 5)
+
+int sequencer_make_script(struct repository *r, FILE *out, int argc,
+ const char **argv,
unsigned flags);
int sequencer_add_exec_commands(struct repository *r, const char *command);
--
2.20.1
^ permalink raw reply related
* [PATCH v4 04/16] sequencer: refactor transform_todos() to work on a todo_list
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This refactors transform_todos() to work on a todo_list. The function
is renamed todo_list_transform().
As rebase -p still need to check the todo list from the disk, a new
function is introduced, transform_todo_file(). It is still used by
complete_action() and edit_todo_list() for now, but they will be
replaced in a future commit.
todo_list_transform() is not a static function, because it will be used
by edit_todo_list() from rebase-interactive.c in a future commit.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
builtin/rebase--interactive.c | 2 +-
rebase-interactive.c | 4 +--
sequencer.c | 54 +++++++++++++++++++++++------------
sequencer.h | 4 ++-
4 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index dd2a55ab1d..0898eb4c59 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -253,7 +253,7 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
}
case SHORTEN_OIDS:
case EXPAND_OIDS:
- ret = transform_todos(the_repository, flags);
+ ret = transform_todo_file(the_repository, flags);
break;
case CHECK_TODO_LIST:
ret = check_todo_list(the_repository);
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 68aff1dac2..842fa07e7e 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -69,7 +69,7 @@ int edit_todo_list(struct repository *r, unsigned flags)
strbuf_release(&buf);
- transform_todos(r, flags | TODO_LIST_SHORTEN_IDS);
+ transform_todo_file(r, flags | TODO_LIST_SHORTEN_IDS);
if (strbuf_read_file(&buf, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
@@ -85,7 +85,7 @@ int edit_todo_list(struct repository *r, unsigned flags)
if (launch_sequence_editor(todo_file, NULL, NULL))
return -1;
- transform_todos(r, flags & ~(TODO_LIST_SHORTEN_IDS));
+ transform_todo_file(r, flags & ~(TODO_LIST_SHORTEN_IDS));
return 0;
}
diff --git a/sequencer.c b/sequencer.c
index 61be04ccc5..6b4eb7a9ba 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4501,27 +4501,18 @@ int sequencer_add_exec_commands(struct repository *r,
return i;
}
-int transform_todos(struct repository *r, unsigned flags)
+void todo_list_transform(struct repository *r, struct todo_list *todo_list,
+ unsigned flags)
{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
struct strbuf buf = STRBUF_INIT;
struct todo_item *item;
int i;
- if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
- return error(_("could not read '%s'."), todo_file);
-
- if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
- todo_list_release(&todo_list);
- return error(_("unusable todo list: '%s'"), todo_file);
- }
-
- for (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {
+ for (item = todo_list->items, i = 0; i < todo_list->nr; i++, item++) {
/* if the item is not a command write it and continue */
if (item->command >= TODO_COMMENT) {
strbuf_addf(&buf, "%.*s\n", item->arg_len,
- todo_list.buf.buf + item->arg_offset);
+ todo_list->buf.buf + item->arg_offset);
continue;
}
@@ -4552,12 +4543,39 @@ int transform_todos(struct repository *r, unsigned flags)
strbuf_addch(&buf, '\n');
else
strbuf_addf(&buf, " %.*s\n", item->arg_len,
- todo_list.buf.buf + item->arg_offset);
+ todo_list->buf.buf + item->arg_offset);
}
- i = write_message(buf.buf, buf.len, todo_file, 0);
+ strbuf_reset(&todo_list->buf);
+ strbuf_add(&todo_list->buf, buf.buf, buf.len);
+ strbuf_release(&buf);
+
+ if (todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list))
+ BUG("unusable todo list");
+}
+
+int transform_todo_file(struct repository *r, unsigned flags)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res;
+
+ if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
+ todo_list_release(&todo_list);
+ return error(_("unusable todo list: '%s'"), todo_file);
+ }
+
+ todo_list_transform(r, &todo_list, flags);
+
+ res = write_message(todo_list.buf.buf, todo_list.buf.len, todo_file, 0);
todo_list_release(&todo_list);
- return i;
+
+ if (res)
+ return error_errno(_("could not write '%s'."), todo_file);
+ return 0;
}
enum missing_commit_check_level get_missing_commit_check_level(void)
@@ -4819,7 +4837,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return error(_("could not copy '%s' to '%s'."), todo_file,
rebase_path_todo_backup());
- if (transform_todos(r, flags | TODO_LIST_SHORTEN_IDS))
+ if (transform_todo_file(r, flags | TODO_LIST_SHORTEN_IDS))
return error(_("could not transform the todo list"));
strbuf_reset(buf);
@@ -4848,7 +4866,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return -1;
}
- if (transform_todos(r, flags & ~(TODO_LIST_SHORTEN_IDS)))
+ if (transform_todo_file(r, flags & ~(TODO_LIST_SHORTEN_IDS)))
return error(_("could not transform the todo list"));
if (opts->allow_ff && skip_unnecessary_picks(r, &oid))
diff --git a/sequencer.h b/sequencer.h
index 70a896e756..53ffe7371c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -121,6 +121,8 @@ struct todo_list {
int todo_list_parse_insn_buffer(struct repository *r, char *buf,
struct todo_list *todo_list);
+void todo_list_transform(struct repository *r, struct todo_list *todo_list,
+ unsigned flags);
void todo_list_release(struct todo_list *todo_list);
/* Call this to setup defaults before parsing command line options */
@@ -146,7 +148,7 @@ int sequencer_make_script(struct repository *repo, FILE *out,
unsigned flags);
int sequencer_add_exec_commands(struct repository *r, const char *command);
-int transform_todos(struct repository *r, unsigned flags);
+int transform_todo_file(struct repository *r, unsigned flags);
enum missing_commit_check_level get_missing_commit_check_level(void);
int check_todo_list(struct repository *r);
int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 03/16] sequencer: remove the 'arg' field from todo_item
From: Alban Gruin @ 2018-12-29 16:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
The 'arg' field of todo_item used to store the address of the first byte
of the parameter of a command in a todo list. It was associated with
the length of the parameter (the 'arg_len' field).
This replaces the 'arg' field by 'arg_offset'. This new field does not
store the address of the parameter, but the position of the first
character of the parameter in the buffer.
This will prevent todo_list_add_exec_commands() from having to do awful
pointer arithmetics when growing the todo list buffer.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
This is a new commit.
sequencer.c | 61 ++++++++++++++++++++++++++++-------------------------
sequencer.h | 4 ++--
2 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5b84a20532..61be04ccc5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1950,7 +1950,7 @@ static struct todo_item *append_new_todo(struct todo_list *todo_list)
}
static int parse_insn_line(struct repository *r, struct todo_item *item,
- const char *bol, char *eol)
+ const char *buf, const char *bol, char *eol)
{
struct object_id commit_oid;
char *end_of_object_name;
@@ -1964,7 +1964,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
if (bol == eol || *bol == '\r' || *bol == comment_line_char) {
item->command = TODO_COMMENT;
item->commit = NULL;
- item->arg = bol;
+ item->arg_offset = bol - buf;
item->arg_len = eol - bol;
return 0;
}
@@ -1991,7 +1991,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
return error(_("%s does not accept arguments: '%s'"),
command_to_string(item->command), bol);
item->commit = NULL;
- item->arg = bol;
+ item->arg_offset = bol - buf;
item->arg_len = eol - bol;
return 0;
}
@@ -2003,7 +2003,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
if (item->command == TODO_EXEC || item->command == TODO_LABEL ||
item->command == TODO_RESET) {
item->commit = NULL;
- item->arg = bol;
+ item->arg_offset = bol - buf;
item->arg_len = (int)(eol - bol);
return 0;
}
@@ -2017,7 +2017,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
} else {
item->flags |= TODO_EDIT_MERGE_MSG;
item->commit = NULL;
- item->arg = bol;
+ item->arg_offset = bol - buf;
item->arg_len = (int)(eol - bol);
return 0;
}
@@ -2029,8 +2029,9 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
status = get_oid(bol, &commit_oid);
*end_of_object_name = saved;
- item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
- item->arg_len = (int)(eol - item->arg);
+ bol = end_of_object_name + strspn(end_of_object_name, " \t");
+ item->arg_offset = bol - buf;
+ item->arg_len = (int)(eol - bol);
if (status < 0)
return -1;
@@ -2058,11 +2059,11 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
item = append_new_todo(todo_list);
item->offset_in_buf = p - todo_list->buf.buf;
- if (parse_insn_line(r, item, p, eol)) {
+ if (parse_insn_line(r, item, buf, p, eol)) {
res = error(_("invalid line %d: %.*s"),
i, (int)(eol - p), p);
item->command = TODO_COMMENT + 1;
- item->arg = p;
+ item->arg_offset = p - buf;
item->arg_len = (int)(eol - p);
item->commit = NULL;
}
@@ -2402,7 +2403,7 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
item->command = command;
item->commit = commit;
- item->arg = NULL;
+ item->arg_offset = 0;
item->arg_len = 0;
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, &subject);
@@ -3438,6 +3439,8 @@ static int pick_commits(struct repository *r,
while (todo_list->current < todo_list->nr) {
struct todo_item *item = todo_list->items + todo_list->current;
+ const char *arg = todo_list->buf.buf + item->arg_offset;
+
if (save_todo(todo_list, opts))
return -1;
if (is_rebase_i(opts)) {
@@ -3488,10 +3491,9 @@ static int pick_commits(struct repository *r,
fprintf(stderr,
_("Stopped at %s... %.*s\n"),
short_commit_name(commit),
- item->arg_len, item->arg);
+ item->arg_len, arg);
return error_with_patch(r, commit,
- item->arg, item->arg_len, opts, res,
- !res);
+ arg, item->arg_len, opts, res, !res);
}
if (is_rebase_i(opts) && !res)
record_in_rewritten(&item->commit->object.oid,
@@ -3500,7 +3502,7 @@ static int pick_commits(struct repository *r,
if (res == 1)
intend_to_amend();
return error_failed_squash(r, item->commit, opts,
- item->arg_len, item->arg);
+ item->arg_len, arg);
} else if (res && is_rebase_i(opts) && item->commit) {
int to_amend = 0;
struct object_id oid;
@@ -3519,16 +3521,16 @@ static int pick_commits(struct repository *r,
to_amend = 1;
return res | error_with_patch(r, item->commit,
- item->arg, item->arg_len, opts,
+ arg, item->arg_len, opts,
res, to_amend);
}
} else if (item->command == TODO_EXEC) {
- char *end_of_arg = (char *)(item->arg + item->arg_len);
+ char *end_of_arg = (char *)(arg + item->arg_len);
int saved = *end_of_arg;
struct stat st;
*end_of_arg = '\0';
- res = do_exec(r, item->arg);
+ res = do_exec(r, arg);
*end_of_arg = saved;
/* Reread the todo file if it has changed. */
@@ -3545,14 +3547,14 @@ static int pick_commits(struct repository *r,
todo_list->current = -1;
}
} else if (item->command == TODO_LABEL) {
- if ((res = do_label(r, item->arg, item->arg_len)))
+ if ((res = do_label(r, arg, item->arg_len)))
reschedule = 1;
} else if (item->command == TODO_RESET) {
- if ((res = do_reset(r, item->arg, item->arg_len, opts)))
+ if ((res = do_reset(r, arg, item->arg_len, opts)))
reschedule = 1;
} else if (item->command == TODO_MERGE) {
if ((res = do_merge(r, item->commit,
- item->arg, item->arg_len,
+ arg, item->arg_len,
item->flags, opts)) < 0)
reschedule = 1;
else if (item->commit)
@@ -3561,9 +3563,8 @@ static int pick_commits(struct repository *r,
if (res > 0)
/* failed with merge conflicts */
return error_with_patch(r, item->commit,
- item->arg,
- item->arg_len, opts,
- res, 0);
+ arg, item->arg_len,
+ opts, res, 0);
} else if (!is_noop(item->command))
return error(_("unknown command %d"), item->command);
@@ -3578,9 +3579,8 @@ static int pick_commits(struct repository *r,
if (item->commit)
return error_with_patch(r,
item->commit,
- item->arg,
- item->arg_len, opts,
- res, 0);
+ arg, item->arg_len,
+ opts, res, 0);
}
todo_list->current++;
@@ -4520,7 +4520,8 @@ int transform_todos(struct repository *r, unsigned flags)
for (item = todo_list.items, i = 0; i < todo_list.nr; i++, item++) {
/* if the item is not a command write it and continue */
if (item->command >= TODO_COMMENT) {
- strbuf_addf(&buf, "%.*s\n", item->arg_len, item->arg);
+ strbuf_addf(&buf, "%.*s\n", item->arg_len,
+ todo_list.buf.buf + item->arg_offset);
continue;
}
@@ -4550,7 +4551,8 @@ int transform_todos(struct repository *r, unsigned flags)
if (!item->arg_len)
strbuf_addch(&buf, '\n');
else
- strbuf_addf(&buf, " %.*s\n", item->arg_len, item->arg);
+ strbuf_addf(&buf, " %.*s\n", item->arg_len,
+ todo_list.buf.buf + item->arg_offset);
}
i = write_message(buf.buf, buf.len, todo_file, 0);
@@ -4626,7 +4628,8 @@ int check_todo_list(struct repository *r)
if (commit && !*commit_seen_at(&commit_seen, commit)) {
strbuf_addf(&missing, " - %s %.*s\n",
short_commit_name(commit),
- item->arg_len, item->arg);
+ item->arg_len,
+ todo_list.buf.buf + item->arg_offset);
*commit_seen_at(&commit_seen, commit) = 1;
}
}
diff --git a/sequencer.h b/sequencer.h
index 7dc4d8946b..70a896e756 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -104,9 +104,9 @@ struct todo_item {
enum todo_command command;
struct commit *commit;
unsigned int flags;
- const char *arg;
int arg_len;
- size_t offset_in_buf;
+ /* The offset of the command and its argument in the strbuf */
+ size_t offset_in_buf, arg_offset;
};
struct todo_list {
--
2.20.1
^ permalink raw reply related
* [PATCH v4 02/16] sequencer: make the todo_list structure public
From: Alban Gruin @ 2018-12-29 16:03 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This makes the structures todo_list and todo_item, and the functions
todo_list_release() and parse_insn_buffer(), accessible outside of
sequencer.c.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
Some changes were introduced because of the conflict resolution with
nd/the-index. They mostly consist of adding a parameter ("r") when
calling todo_list_parse_insn_buffer.
sequencer.c | 69 ++++++++++-------------------------------------------
sequencer.h | 50 ++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 57 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a7afaf6882..5b84a20532 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1460,32 +1460,6 @@ static int allow_empty(struct repository *r,
return 1;
}
-/*
- * 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 {
- /* commands that handle commits */
- TODO_PICK = 0,
- TODO_REVERT,
- TODO_EDIT,
- TODO_REWORD,
- TODO_FIXUP,
- TODO_SQUASH,
- /* commands that do something else than handling a single commit */
- TODO_EXEC,
- TODO_BREAK,
- TODO_LABEL,
- TODO_RESET,
- TODO_MERGE,
- /* commands that do nothing but are counted for reporting progress */
- TODO_NOOP,
- TODO_DROP,
- /* comments (not counted for reporting progress) */
- TODO_COMMENT
-};
-
static struct {
char c;
const char *str;
@@ -1962,26 +1936,7 @@ enum todo_item_flags {
TODO_EDIT_MERGE_MSG = 1
};
-struct todo_item {
- enum todo_command command;
- struct commit *commit;
- unsigned int flags;
- const char *arg;
- int arg_len;
- size_t offset_in_buf;
-};
-
-struct todo_list {
- struct strbuf buf;
- struct todo_item *items;
- int nr, alloc, current;
- int done_nr, total_nr;
- struct stat_data stat;
-};
-
-#define TODO_LIST_INIT { STRBUF_INIT }
-
-static void todo_list_release(struct todo_list *todo_list)
+void todo_list_release(struct todo_list *todo_list)
{
strbuf_release(&todo_list->buf);
FREE_AND_NULL(todo_list->items);
@@ -2084,8 +2039,8 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
return !item->commit;
}
-static int parse_insn_buffer(struct repository *r, char *buf,
- struct todo_list *todo_list)
+int todo_list_parse_insn_buffer(struct repository *r, char *buf,
+ struct todo_list *todo_list)
{
struct todo_item *item;
char *p = buf, *next_p;
@@ -2184,7 +2139,7 @@ static int read_populate_todo(struct repository *r,
return error(_("could not stat '%s'"), todo_file);
fill_stat_data(&todo_list->stat, &st);
- res = parse_insn_buffer(r, todo_list->buf.buf, todo_list);
+ res = todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list);
if (res) {
if (is_rebase_i(opts))
return error(_("please fix this using "
@@ -2215,7 +2170,7 @@ static int read_populate_todo(struct repository *r,
FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w");
if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 &&
- !parse_insn_buffer(r, done.buf.buf, &done))
+ !todo_list_parse_insn_buffer(r, done.buf.buf, &done))
todo_list->done_nr = count_commands(&done);
else
todo_list->done_nr = 0;
@@ -4501,7 +4456,7 @@ int sequencer_add_exec_commands(struct repository *r,
if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
- if (parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
todo_list_release(&todo_list);
return error(_("unusable todo list: '%s'"), todo_file);
}
@@ -4557,7 +4512,7 @@ int transform_todos(struct repository *r, unsigned flags)
if (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
- if (parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list)) {
todo_list_release(&todo_list);
return error(_("unusable todo list: '%s'"), todo_file);
}
@@ -4643,7 +4598,7 @@ int check_todo_list(struct repository *r)
goto leave_check;
}
advise_to_edit_todo = res =
- parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
+ todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
goto leave_check;
@@ -4662,7 +4617,7 @@ int check_todo_list(struct repository *r)
goto leave_check;
}
strbuf_release(&todo_file);
- res = !!parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
+ res = !!todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list);
/* Find commits in git-rebase-todo.backup yet unseen */
for (i = todo_list.nr - 1; i >= 0; i--) {
@@ -4744,7 +4699,7 @@ static int skip_unnecessary_picks(struct repository *r, struct object_id *output
if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
return -1;
- if (parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
todo_list_release(&todo_list);
return -1;
}
@@ -4832,7 +4787,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
if (strbuf_read_file(buf, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
- if (parse_insn_buffer(r, buf->buf, &todo_list)) {
+ if (todo_list_parse_insn_buffer(r, buf->buf, &todo_list)) {
todo_list_release(&todo_list);
return error(_("unusable todo list: '%s'"), todo_file);
}
@@ -4940,7 +4895,7 @@ int rearrange_squash(struct repository *r)
if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
return -1;
- if (parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
+ if (todo_list_parse_insn_buffer(r, todo_list.buf.buf, &todo_list) < 0) {
todo_list_release(&todo_list);
return -1;
}
diff --git a/sequencer.h b/sequencer.h
index d2c18edd3a..7dc4d8946b 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -73,6 +73,56 @@ enum missing_commit_check_level {
int write_message(const void *buf, size_t len, const char *filename,
int append_eol);
+/*
+ * Note that ordering matters in this enum. Not only must it match the mapping
+ * of todo_command_info (in sequencer.c), 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 {
+ /* commands that handle commits */
+ TODO_PICK = 0,
+ TODO_REVERT,
+ TODO_EDIT,
+ TODO_REWORD,
+ TODO_FIXUP,
+ TODO_SQUASH,
+ /* commands that do something else than handling a single commit */
+ TODO_EXEC,
+ TODO_BREAK,
+ TODO_LABEL,
+ TODO_RESET,
+ TODO_MERGE,
+ /* commands that do nothing but are counted for reporting progress */
+ TODO_NOOP,
+ TODO_DROP,
+ /* comments (not counted for reporting progress) */
+ TODO_COMMENT
+};
+
+struct todo_item {
+ enum todo_command command;
+ struct commit *commit;
+ unsigned int flags;
+ const char *arg;
+ int arg_len;
+ size_t offset_in_buf;
+};
+
+struct todo_list {
+ struct strbuf buf;
+ struct todo_item *items;
+ int nr, alloc, current;
+ int done_nr, total_nr;
+ struct stat_data stat;
+};
+
+#define TODO_LIST_INIT { STRBUF_INIT }
+
+int todo_list_parse_insn_buffer(struct repository *r, char *buf,
+ struct todo_list *todo_list);
+void todo_list_release(struct todo_list *todo_list);
+
/* Call this to setup defaults before parsing command line options */
void sequencer_init_config(struct replay_opts *opts);
int sequencer_pick_revisions(struct repository *repo,
--
2.20.1
^ permalink raw reply related
* [PATCH v4 01/16] sequencer: changes in parse_insn_buffer()
From: Alban Gruin @ 2018-12-29 16:03 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181229160413.19333-1-alban.gruin@gmail.com>
This clears the number of items of a todo_list before parsing it to
allow to parse the same list multiple times without issues. As its
items are not dynamically allocated, or don’t need to allocate memory,
no additionnal memory management is required here.
Furthermore, if a line is invalid, the type of the corresponding
command is set to a garbage value, and its argument is defined properly.
This will allow to recreate the text of a todo list from its commands,
even if one of them is incorrect.
Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
---
The full diff changed due to the conflict resolution with nd/the-index,
but the "core change" is the same as in the v3.
sequencer.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index d726f77e11..a7afaf6882 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2091,6 +2091,8 @@ static int parse_insn_buffer(struct repository *r, char *buf,
char *p = buf, *next_p;
int i, res = 0, fixup_okay = file_exists(rebase_path_done());
+ todo_list->current = todo_list->nr = 0;
+
for (i = 1; *p; i++, p = next_p) {
char *eol = strchrnul(p, '\n');
@@ -2104,7 +2106,10 @@ static int parse_insn_buffer(struct repository *r, char *buf,
if (parse_insn_line(r, item, p, eol)) {
res = error(_("invalid line %d: %.*s"),
i, (int)(eol - p), p);
- item->command = TODO_NOOP;
+ item->command = TODO_COMMENT + 1;
+ item->arg = p;
+ item->arg_len = (int)(eol - p);
+ item->commit = NULL;
}
if (fixup_okay)
--
2.20.1
^ permalink raw reply related
* [PATCH v4 00/16] sequencer: refactor functions working on a todo_list
From: Alban Gruin @ 2018-12-29 16:03 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano, Alban Gruin
In-Reply-To: <20181109080805.6350-1-alban.gruin@gmail.com>
At the center of the "interactive" part of the interactive rebase lies
the todo list. When the user starts an interactive rebase, a todo list
is generated, presented to the user (who then edits it using a text
editor), read back, and then is checked and processed before the actual
rebase takes place.
Some of this processing includes adding execs commands, reordering
fixup! and squash! commits, and checking if no commits were accidentally
dropped by the user.
Before I converted the interactive rebase in C, these functions were
called by git-rebase--interactive.sh through git-rebase--helper. Since
the only way to pass around a large amount of data between a shell
script and a C program is to use a file (or any declination of a file),
the functions that checked and processed the todo list were directly
working on a file, the same file that the user edited.
During the conversion, I did not address this issue, which lead to a
complete_action() that reads the todo list file, does some computation
based on its content, and writes it back to the disk, several times in
the same function.
As it is not an efficient way to handle a data structure, this patch
series refactor the functions that processes the todo list to work on a
todo_list structure instead of reading it from the disk.
Some commits consists in modifying edit_todo_list() (initially used by
--edit-todo) to handle the initial edition of the todo list, to increase
code sharing.
This is based on nd/the-index (36e7ed69de, "rebase-interactive.c: remove
the_repository references"), as it introduced a lot of conflicts. The
result does not conflict with pu (e31bc98f4b, "Merge branch
'md/list-objects-filter-by-depth' into pu").
Changes since v3:
- Replacing the 'arg' field from todo_item by 'arg_offset' to avoid
dealing with pointers on the todo list buffer in
todo_list_add_exec_commands(). This has led to some additionnal
changes.
- Rewording some commits.
- Dropping the commit "sequencer: fix a call to error() in
transform_todo_file()". The call to error() after reading the todo
file is replaced by error_errno() in "sequencer: refactor
transform_todos() to work on a todo_list". The same change has been
applied to sequencer_add_exec_commands() in "sequencer: refactor
sequencer_add_exec_commands() to work on a todo_list".
- transform_todo_file(), sequencer_add_exec_commands() and
rearrange_squash_in_todo_file() now print an error if they fail to
write to the todo file.
- A lot of changes were introduced by the conflict resolution with
nd/the-index.
Alban Gruin (16):
sequencer: changes in parse_insn_buffer()
sequencer: make the todo_list structure public
sequencer: remove the 'arg' field from todo_item
sequencer: refactor transform_todos() to work on a todo_list
sequencer: introduce todo_list_write_to_file()
sequencer: refactor check_todo_list() to work on a todo_list
sequencer: refactor sequencer_add_exec_commands() to work on a
todo_list
sequencer: refactor rearrange_squash() to work on a todo_list
sequencer: make sequencer_make_script() write its script to a strbuf
sequencer: change complete_action() to use the refactored functions
sequencer: refactor skip_unnecessary_picks() to work on a todo_list
rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
rebase-interactive: append_todo_help() changes
rebase-interactive: rewrite edit_todo_list() to handle the initial
edit
sequencer: use edit_todo_list() in complete_action()
rebase--interactive: move transform_todo_file() to
rebase--interactive.c
builtin/rebase--interactive.c | 90 +++--
rebase-interactive.c | 143 +++++--
rebase-interactive.h | 8 +-
sequencer.c | 677 +++++++++++++---------------------
sequencer.h | 82 +++-
5 files changed, 525 insertions(+), 475 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH/RFC v1 1/1] Support working-tree-encoding "UTF-16LE-BOM"
From: Adrián Gimeno Balaguer @ 2018-12-29 15:48 UTC (permalink / raw)
To: tboegi, git
In-Reply-To: <CADN+U_OccLuLN7_0rjikDgLT+Zvt8hka-=xsnVVLJORjYzP78Q@mail.gmail.com>
Hello again.
I appreciate the grown interest in this issue.
Torsten, may I know what is the benefit on your code? My PR solved it
by only tweaking the utf8.c's function 'has_prohibited_utf_bom', which
is likely the shortest way:
https://github.com/git/git/pull/550/files
In order to make sure everything is clear, here is a case list of
current Git behaviour and new one after my PR, regarding this issue.
Current behaviour:
- Placing 'test.txt working-tree-encoding=UTF-16' for a new test.txt
file with either UTF-16 BE or LE BOM, and comitting everything -> The
file gets re-encoded from UTF-8 (as stored internally), to UTF-16 and
the default system/libiconv endianness -> Problem (as long as user
required the opposite endianness for any reason on his project). As a
note, user can see however human-readable diffs on that file.
- Placing 'test.txt working-tree-encoding=UTF-16LE' or 'test.txt
working-tree-encoding=UTF-16BE' for a new test.txt file with either
UTF-16 BE or LE BOM, and comitting everything: we assume user is doing
this because he requires that exact endianness, thus he writes it in
order to attempt preserving it -> Git prohibites commiting it, also no
human-readable diff is shown in the diff viewer/tool being used, but
file is simply shown as binary.
New behaviour:
- Just got too lazy to repeat it all over, read my PR description:
https://github.com/git/git/pull/550
- Git translations may need to be tweaked to in order to be consistent
with new behaviour.
Thanks for your attention.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2018, #02; Fri, 28)
From: Denton Liu @ 2018-12-29 12:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqh8explya.fsf@gitster-ct.c.googlers.com>
On Fri, Dec 28, 2018 at 10:04:13AM -0800, Junio C Hamano wrote:
> * dl/remote-save-to-push (2018-12-11) 1 commit
> - remote: add --save-to-push option to git remote set-url
>
> "git remote set-url" learned a new option that moves existing value
> of the URL field to pushURL field of the remote before replacing
> the URL field with a new value.
>
> I am personally not yet quite convinced if this is worth pursuing.
Hi Junio, just wondering from our previous discussions, is this comment
outdated or is it still valid?
Thanks,
-Denton
^ permalink raw reply
* [PATCH/RFC v1 1/1] Support working-tree-encoding "UTF-16LE-BOM"
From: tboegi @ 2018-12-29 11:09 UTC (permalink / raw)
To: git, adrigibal; +Cc: Torsten Bögershausen
In-Reply-To: <CADN+U_PUfnYWb-wW6drRANv-ZaYBEk3gWHc7oJtxohA5Vc3NEg@mail.gmail.com>
From: Torsten Bögershausen <tboegi@web.de>
Users who want UTF-16 files in the working tree set the .gitattributes
like this:
test.txt working-tree-encoding=UTF-16
After a checkout, the resulting file has a BOM and is encoded in "UTF-16".
The unicode standard allows both little- and big-endianess (LE/BE) for
those files, the BOM will tell which one is used inside the file.
iconv seems to prefer the BE version.
Not all users under Windows are happy with this when tools are not fully
unicode aware and don't digest the BE version at all.
Today there is no name for "UTF-16 with BOM, little endian please".
Introduce "UTF-16LE-BOM".
Rported-by: Adrián Gimeno Balaguer <adrigibal@gmail.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
This feels like an RFC at the moment - please comment.
Using UTF-16 in the way "UTF-16LE-BOM" is used in this patch
could be an alternative - simply produce UTF-16 in LE version
under Git - this could make people using Git happy as well.
Documentation/gitattributes.txt | 4 +--
compat/precompose_utf8.c | 2 +-
t/t0028-working-tree-encoding.sh | 12 ++++++++-
utf8.c | 42 ++++++++++++++++++++++++--------
utf8.h | 2 +-
5 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index b8392fc330..4a88ab8be7 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -343,13 +343,13 @@ automatic line ending conversion based on your platform.
------------------------
Use the following attributes if your '*.ps1' files are UTF-16 little
-endian encoded without BOM and you want Git to use Windows line endings
+endian encoded with BOM and you want Git to use Windows line endings
in the working directory. Please note, it is highly recommended to
explicitly define the line endings with `eol` if the `working-tree-encoding`
attribute is used to avoid ambiguity.
------------------------
-*.ps1 text working-tree-encoding=UTF-16LE eol=CRLF
+*.ps1 text working-tree-encoding=UTF-16LE-BOM eol=CRLF
------------------------
You can get a list of all available encodings on your platform with the
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index de61c15d34..136250fbf6 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -79,7 +79,7 @@ void precompose_argv(int argc, const char **argv)
size_t namelen;
oldarg = argv[i];
if (has_non_ascii(oldarg, (size_t)-1, &namelen)) {
- newarg = reencode_string_iconv(oldarg, namelen, ic_precompose, NULL);
+ newarg = reencode_string_iconv(oldarg, namelen, ic_precompose, 0, NULL);
if (newarg)
argv[i] = newarg;
}
diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
index 7e87b5a200..e58ecbfc44 100755
--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -11,9 +11,12 @@ test_expect_success 'setup test files' '
text="hallo there!\ncan you read me?" &&
echo "*.utf16 text working-tree-encoding=utf-16" >.gitattributes &&
+ echo "*.utf16lebom text working-tree-encoding=UTF-16LE-BOM" >>.gitattributes &&
printf "$text" >test.utf8.raw &&
printf "$text" | iconv -f UTF-8 -t UTF-16 >test.utf16.raw &&
printf "$text" | iconv -f UTF-8 -t UTF-32 >test.utf32.raw &&
+ printf "\377\376" >test.utf16lebom.raw &&
+ printf "$text" | iconv -f UTF-8 -t UTF-32LE >>test.utf16lebom.raw &&
# Line ending tests
printf "one\ntwo\nthree\n" >lf.utf8.raw &&
@@ -32,7 +35,8 @@ test_expect_success 'setup test files' '
# Add only UTF-16 file, we will add the UTF-32 file later
cp test.utf16.raw test.utf16 &&
cp test.utf32.raw test.utf32 &&
- git add .gitattributes test.utf16 &&
+ cp test.utf16lebom.raw test.utf16lebom &&
+ git add .gitattributes test.utf16 test.utf16lebom &&
git commit -m initial
'
@@ -51,6 +55,12 @@ test_expect_success 're-encode to UTF-16 on checkout' '
test_cmp_bin test.utf16.raw test.utf16
'
+test_expect_success 're-encode to UTF-16-LE-BOM on checkout' '
+ rm test.utf16lebom &&
+ git checkout test.utf16lebom &&
+ test_cmp_bin test.utf16lebom.raw test.utf16lebom
+'
+
test_expect_success 'check $GIT_DIR/info/attributes support' '
test_when_finished "rm -f test.utf32.git" &&
test_when_finished "git reset --hard HEAD" &&
diff --git a/utf8.c b/utf8.c
index eb78587504..83824dc2f4 100644
--- a/utf8.c
+++ b/utf8.c
@@ -4,6 +4,11 @@
/* This code is originally from http://www.cl.cam.ac.uk/~mgk25/ucs/ */
+static const char utf16_be_bom[] = {'\xFE', '\xFF'};
+static const char utf16_le_bom[] = {'\xFF', '\xFE'};
+static const char utf32_be_bom[] = {'\0', '\0', '\xFE', '\xFF'};
+static const char utf32_le_bom[] = {'\xFF', '\xFE', '\0', '\0'};
+
struct interval {
ucs_char_t first;
ucs_char_t last;
@@ -470,16 +475,17 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
#else
typedef char * iconv_ibp;
#endif
-char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, size_t *outsz_p)
+char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv,
+ size_t bom_len, size_t *outsz_p)
{
size_t outsz, outalloc;
char *out, *outpos;
iconv_ibp cp;
outsz = insz;
- outalloc = st_add(outsz, 1); /* for terminating NUL */
+ outalloc = st_add(outsz, 1 + bom_len); /* for terminating NUL */
out = xmalloc(outalloc);
- outpos = out;
+ outpos = out + bom_len;
cp = (iconv_ibp)in;
while (1) {
@@ -540,10 +546,30 @@ char *reencode_string_len(const char *in, size_t insz,
{
iconv_t conv;
char *out;
+ const char *bom_str = NULL;
+ size_t bom_len = 0;
if (!in_encoding)
return NULL;
+ /* UTF-16LE-BOM is the same as UTF-16 for reading */
+ if (same_utf_encoding("UTF-16LE-BOM", in_encoding))
+ in_encoding = "UTF-16";
+
+ /*
+ * For writing, UTF-16 iconv typically creates "UTF-16BE-BOM"
+ * Some users under Windows want the little endian version
+ */
+ if (same_utf_encoding("UTF-16LE-BOM", out_encoding)) {
+ bom_str = utf16_le_bom;
+ bom_len = sizeof(utf16_le_bom);
+ out_encoding = "UTF-16LE";
+ } else if (same_utf_encoding("UTF-16BE-BOM", out_encoding)) {
+ bom_str = utf16_be_bom;
+ bom_len = sizeof(utf16_be_bom);
+ out_encoding = "UTF-16BE";
+ }
+
conv = iconv_open(out_encoding, in_encoding);
if (conv == (iconv_t) -1) {
in_encoding = fallback_encoding(in_encoding);
@@ -553,9 +579,10 @@ char *reencode_string_len(const char *in, size_t insz,
if (conv == (iconv_t) -1)
return NULL;
}
-
- out = reencode_string_iconv(in, insz, conv, outsz);
+ out = reencode_string_iconv(in, insz, conv, bom_len, outsz);
iconv_close(conv);
+ if (out && bom_str && bom_len)
+ memcpy(out, bom_str, bom_len);
return out;
}
#endif
@@ -566,11 +593,6 @@ static int has_bom_prefix(const char *data, size_t len,
return data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);
}
-static const char utf16_be_bom[] = {'\xFE', '\xFF'};
-static const char utf16_le_bom[] = {'\xFF', '\xFE'};
-static const char utf32_be_bom[] = {'\0', '\0', '\xFE', '\xFF'};
-static const char utf32_le_bom[] = {'\xFF', '\xFE', '\0', '\0'};
-
int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)
{
return (
diff --git a/utf8.h b/utf8.h
index edea55e093..84efbfcb1f 100644
--- a/utf8.h
+++ b/utf8.h
@@ -27,7 +27,7 @@ void strbuf_utf8_replace(struct strbuf *sb, int pos, int width,
#ifndef NO_ICONV
char *reencode_string_iconv(const char *in, size_t insz,
- iconv_t conv, size_t *outsz);
+ iconv_t conv, size_t bom_len, size_t *outsz);
char *reencode_string_len(const char *in, size_t insz,
const char *out_encoding,
const char *in_encoding,
--
2.20.1.2.gb21ebb671b
^ permalink raw reply related
* Re: RFE: version-controlled merge rules
From: hpa @ 2018-12-29 9:14 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Nieder; +Cc: git
In-Reply-To: <xmqqo995pvmc.fsf@gitster-ct.c.googlers.com>
On December 28, 2018 6:35:23 AM PST, Junio C Hamano <gitster@pobox.com> wrote:
>Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> The main issue I see is that this would make it a little *too* easy
>to
>> run arbitrary code on the user's machine. Build systems often
>already
>> lead to that, but users are more familiar with the risks for build
>> than for version control.
>>
>> See [1] for some related discussion.
>>
>> That said, using the include.path feature (see git-config(1)), it's
>> possible to do something similar:
>>
>> [include]
>> path = ../.gitconfig
>>
>> Thanks and hope that helps,
>
>The issue the arrangement to specify what kind of files they are in
>the attribute system and to specify what exact commands to be run in
>the configuration addresses is twofold. The security issue is one
>and poking a hole with include.path mechanism is probably OK as
>there is end-user consent, but I tend to agree that a similar risk
>already exists by a project shipping Makefile et al.
>
>There is the other side of the issue.
>
>The arrangement allows project not to be monoculture by leaving the
>exact command sequence to use on the kind of files (specified by the
>project with the attribute system) up to the end-user in their
>configuration. While Peter may feel that sort piped to head may be
>available on all the reasonable UNIX systems, his merge driver would
>not work on other platforms. There already is a similar reliance of
>monoculture by a project shipping Makefile et al, which is an
>interesting parallel.
This is actually a further good reason for doing it this way: it means that more genal drivers can be written using files in the repo, depending on what the baseline of the project is.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 0/4] Allow 'cherry-pick -m 1' for non-merge commits
From: Sergey Organov @ 2018-12-29 9:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqr2e1mmuu.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Sergey Organov <sorganov@gmail.com>, Sergey Organov
> <sorganov@gmail.com> writes:
>
>> Hi Junio,
>>
>> What's the status of these patches?
>
> The status of these patches is "Just updated on the list", as far as
> I am concerned, and its cover letter would have described what's
> improved relative to the previous round better than whatever answer
> I could give here ;-)
Yeah, I need to be more thorough, thanks!
-- Sergey
^ permalink raw reply
* Re: "IMAP IDLE"-like long-polling "git fetch"
From: Eric Wong @ 2018-12-29 6:13 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: git, meta
In-Reply-To: <20181229043858.GA28509@pure.paranoia.local>
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Sat, Dec 29, 2018 at 03:56:21AM +0000, Eric Wong wrote:
> > Hey all, I just added this to the TODO file for public-inbox[1] but
> > obviously it's intended for git.git (meta@public-inbox cc-ed):
> >
> > > +* Contribute something like IMAP IDLE for "git fetch".
> > > + Inboxes (and any git repos) can be kept up-to-date without
> > > + relying on polling.
> >
> > I would've thought somebody had done this by now, but I guess
> > it's dependent on a bunch of things (TLS layer nowadays, maybe
> > HTTP/2), so git-daemon support alone wouldn't cut it...
>
> Polling is not all bad, especially for large repository collections. I'm
> not sure you want to "idle" individual repositories when there's
> thousands of them. We ended up writing grokmirror for replicating
> repo collections using manifest files.
I wasn't intending it for giant sites like korg, but for
individual hackers on their workstations tracking a handful of
projects they follow.
The cost for a hackers' machine would be the same as the current
situation where developers idle on IRC channels for the projects
they're involved in.
> > Anyways, until this is implemented, feel free to continue
> > hammering a way on https://public-inbox.org/git/ with frequent
> > "git fetch". I write C10K servers in my sleep -_-
>
> The archive is also mirrored at
> https://git.kernel.org/pub/scm/public-inbox/vger.kernel.org/git.git, and
> also on kernel.googlesource.com.
Now, I'm wondering if you can make a v2 public-inbox mirror of
git@vger and run it on lore. Converting public-inbox.org/git to
v2 would break things for everybody fetching, right now :<
^ 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