"git diff/log --ws-error-highlight=<kind>" lacked the corresponding
configuration variable to set it by default.
* jc/ws-error-highlight:
diff: introduce diff.wsErrorHighlight option
diff.c: move ws-error-highlight parsing helpers up
diff.c: refactor parse_ws_error_highlight()
t4015: split out the "setup" part of ws-error-highlight test
Overflow is defined for unsigned integers, but not for signed ones.
We could make the ring-buffer index in sha1_to_hex() and
get_pathname() unsigned to be on the safe side to resolve this, but
let's make it explicit that we are wrapping around at whatever the
number of elements the ring-buffer has. The compiler is smart enough
to turn modulus into bitmask for these codepaths that use
ring-buffers of a size that is a power of 2.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of referencing "stash@{n}" explicitly, make it possible to
simply reference as "n". Most users only reference stashes by their
position in the stash stack (what I refer to as the "index" here).
The syntax for the typical stash (stash@{n}) is slightly annoying and
easy to forget, and sometimes difficult to escape properly in a
script. Because of this the capability to do things with the stash by
simply referencing the index is desirable.
This patch includes the superior implementation provided by Øsse Walle
(thanks for that), with a slight change to fix a broken test in the test
suite. I also merged the test scripts as suggested by Jeff King, and
un-wrapped the documentation as suggested by Junio Hamano.
Signed-off-by: Aaron M Watson <watsona4@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
When an MSYS program (such as the bash that drives the test suite)
invokes git on Windows, absolute Unix style paths are transformed into
Windows native absolute paths (drive letter form). However, this
transformation also includes some simplifications that are not just
straight-forward textual substitutions:
- When the path ends in "/.", then the dot is stripped, but not the
directory separator.
- When the path contains "..", then it is optimized away if possible,
e.g., "/c/dir/foo/../bar" becomes "c:/dir/bar".
These additional transformations violate the assumptions of some
submodule path tests. We can avoid them when the input is already a
Windows native path, because then MSYS leaves the path unmolested.
Convert the uses of $PWD to $(pwd); the latter returns a native Windows
path.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This fixes "convert: add filter.<driver>.process option" (edcc8581) on
Windows.
Consider the case of a file that requires filtering and is present in
branch A but not in branch B. If A is the current HEAD and we checkout B
then the following happens:
1. ce_compare_data() opens the file
2. index_fd() detects that the file requires to run a clean filter and
calls index_stream_convert_blob()
4. index_stream_convert_blob() calls convert_to_git_filter_fd()
5. convert_to_git_filter_fd() calls apply_filter() which creates a
new long running filter process (in case it is the first file
of this kind to be filtered)
6. The new filter process inherits all file handles. This is the
default on Linux/OSX and is explicitly defined in the
`CreateProcessW` call in `mingw.c` on Windows.
7. ce_compare_data() closes the file
8. Git unlinks the file as it is not present in B
The unlink operation does not work on Windows because the filter process
has still an open handle to the file. On Linux/OSX the unlink operation
succeeds but the file descriptors still leak into the child process.
Fix this problem by opening files in read-cache with the O_CLOEXEC flag
to ensure that the file descriptor does not remain open in a newly
spawned process similar to 05d1ed6148 ("mingw: ensure temporary file
handles are not inherited by child processes", 2016-08-22).
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All processes that the Git main process spawns inherit the open file
descriptors of the main process. These leaked file descriptors can
cause problems.
Use the O_CLOEXEC flag similar to 05d1ed61 to fix the leaked file
descriptors.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function is meant to be used when reading from files in the
object store, and the original objective was to avoid smudging atime
of loose object files too often, hence its name. Because we'll be
extending its role in the next commit to also arrange the file
descriptors they return auto-closed in the child processes, rename
it to lose "noatime" part that is too specific.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The asciidoctor doc-tool stack does not always respect the 'tab = 8 spaces' rule
expectation, particularly for the Git-for-Windows generated html pages. This
follows on from the 'doc: fix merge-base ASCII art tab spacing' fix.
Use just spaces within the block of the ascii art.
All other *.txt ascii art containing three dashes has been checked.
Asciidoctor correctly formats the other art blocks that do contain tabs.
Signed-off-by: Philip Oakley <philipoakley@iee.org
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ita entries are dropped at tree generation phase. If the entire index
consists of just ita entries, the result would be a a commit with no
entries, which should be caught unless --allow-empty is specified. The
test "!!active_nr" is not sufficient to catch this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If i-t-a entries are present and there is no change between the index
and HEAD i-t-a entries, index_differs_from() still returns "dirty, new
entries" (aka, the resulting commit is not empty), but cache-tree will
skip i-t-a entries and produce the exact same tree of current
commit.
index_differs_from() is supposed to catch this so we can abort
git-commit (unless --no-empty is specified). Update it to optionally
ignore i-t-a entries when doing a diff between the index and HEAD so
that it would return "no change" in this case and abort commit.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The option --ita-invisible-in-index exposes the "ita_invisible_in_index"
diff flag to outside to allow easier experimentation with this new mode.
The "plan" is to make --ita-invisible-in-index default to keep consistent
behavior with 'status' and 'commit', but a bunch other commands like
'apply', 'merge', 'reset'.... need to be taken into consideration as well.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When comparing the index and the working tree to show which paths are
new, and comparing the tree recorded in the HEAD and the index to see if
committing the contents recorded in the index would result in an empty
commit, we would want the former comparison to say "these are new paths"
and the latter to say "there is no change" for paths that are marked as
intent-to-add.
We made a similar attempt at d95d728a ("diff-lib.c: adjust position of
i-t-a entries in diff", 2015-03-16), which redefined the semantics of
these two comparison modes globally, which was a disaster and had to be
reverted at 78cc1a54 ("Revert "diff-lib.c: adjust position of i-t-a
entries in diff"", 2015-06-23).
To make sure we do not repeat the same mistake, introduce a new internal
diffopt option so that this different semantics can be asked for only by
callers that ask it, while making sure other unaudited callers will get
the same comparison result.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are checking the path via path_ok(), we use some
fixed PATH_MAX buffers. We write into them via snprintf(),
so there's no possibility of overflow, but it does mean we
may silently truncate the path, leading to potentially
confusing errors when the partial path does not exist.
We're better off to reject the path explicitly.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When there is a TAP harness consuming the output of our test
scripts, the "--verbose" breaks the output by mingling
test command output with TAP. Because the TAP::Harness
module used by "prove" is fairly lenient, this _usually_
works, but it violates the spec, and things get very
confusing if the commands happen to output a line that looks
like TAP (e.g., the word "ok" on its own line).
Let's detect this situation and complain. Just calling
error() isn't great, though; prove will tell us that the
script failed, but the message doesn't make it through to
the user. Instead, we can use the special TAP signal "Bail
out!". This not only shows the message to the user, but
instructs the harness to stop running the tests entirely.
This is exactly what we want here, as the problem is in the
command-line options, and every test script would produce
the same error.
The result looks like this (the first "Bailout called" line
is in red if prove uses color on your terminal):
$ make GIT_TEST_OPTS='--verbose --tee'
rm -f -r 'test-results'
*** prove ***
Bailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log
FAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log
Makefile:39: recipe for target 'prove' failed
make: *** [prove] Error 255
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now all that is left to do is to actually iterate over the refs
and measure the display width needed to show their abbreviation.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now we have identified three callchains that have a set of refs that
they want to show their <old, new> object names in an aligned output,
we can replace their reference to the constant TRANSPORT_SUMMARY_WIDTH
with a helper function call to transport_summary_width() that takes
the set of ref as a parameter. This step does not yet iterate over
the refs and compute, which is left as an exercise to the readers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The leaf function on the "fetch" side that uses TRANSPORT_SUMMARY_WIDTH
constant is builtin/fetch.c::format_display() and it has two distinct
callchains. The one that reports the primary result of fetch originates
at store_updated_refs(); the other one that reports the pruning of
the remote-tracking refs originates at prune_refs().
Teach these two places to pass summary_width down the callchain,
just like we did for the "push" side in the previous commit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The callchain that originates at transport_print_push_status()
eventually hits a single leaf function, print_ref_status(), that is
used to show from what old object to what new object a ref got
updated, and the width of the part that shows old and new object
names used a constant TRANSPORT_SUMMARY_WIDTH.
Teach the callchain to pass the width down from the top instead.
This allows a future enhancement to compute the necessary display
width before calling down this callchain.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, interpret-trailers requires that a trailer be only on 1 line.
For example:
a: first line
second line
would be interpreted as one trailer line followed by one non-trailer line.
Make interpret-trailers support RFC 822-style folding, treating those
lines as one logical trailer.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, interpret-trailers allows leading whitespace in trailer
lines. This leads to false positives, especially for quoted lines or
bullet lists.
Forbid leading whitespace in trailers.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, interpret-trailers requires all lines of a trailer block to
be trailers (or comments) - if not it would not identify that block as a
trailer block, and thus create its own trailer block, inserting a blank
line. For example:
echo -e "\nSigned-off-by: x\nnot trailer" |
git interpret-trailers --trailer "c: d"
would result in:
Signed-off-by: x
not trailer
c: d
Relax the definition of a trailer block to require that the trailers (i)
are all trailers, or (ii) contain at least one Git-generated trailer and
consists of at least 25% trailers.
Signed-off-by: x
not trailer
c: d
(i) is the existing functionality. (ii) allows arbitrary lines to be
included in trailer blocks, like those in [1], and still allow
interpret-trailers to be used.
[1]
e7d316a02f
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse_trailer function has a few modes of operation, all depending
on whether the separator is present in its input, and if yes, the
separator's position. Some of these modes are failure modes, and these
failure modes are handled differently depending on whether the trailer
line was sourced from a file or from a command-line argument.
Extract a function to find the separator, allowing the invokers of
parse_trailer to determine how to handle the failure modes instead of
making parse_trailer do it. In this function, also take in the list of
separators, so that we can distinguish between command line arguments
(which allow '=' as separator) and file input (which does not allow '='
as separator).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Because we run the tests via "prove", the output from
"--verbose" may interfere with our TAP output. Using
"--verbose-log" solves this while letting us retain our
on-disk log.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "--verbose" option redirects output from arbitrary
test commands to stdout. This is useful for examining the
output manually, like:
./t5547-push-quarantine.sh -v | less
But it also means that the output is intermingled with the
TAP directives, which can confuse a TAP parser like "prove".
This has always been a potential problem, but became an
issue recently when one test happened to output the word
"ok" on a line by itself, which prove interprets as a test
success:
$ prove t5547-push-quarantine.sh :: -v
t5547-push-quarantine.sh .. 1/? To dest.git
* [new branch] HEAD -> master
To dest.git
! [remote rejected] reject -> reject (pre-receive hook declined)
error: failed to push some refs to 'dest.git'
fatal: git cat-file d08c8eba97f4e683ece08654c7c8d2ba0c03b129: bad file
t5547-push-quarantine.sh .. Failed -1/4 subtests
Test Summary Report
-------------------
t5547-push-quarantine.sh (Wstat: 0 Tests: 5 Failed: 0)
Parse errors: Tests out of sequence. Found (2) but expected (3)
Tests out of sequence. Found (3) but expected (4)
Tests out of sequence. Found (4) but expected (5)
Bad plan. You planned 4 tests but ran 5.
Files=1, Tests=5, 0 wallclock secs ( 0.01 usr + 0.01 sys = 0.02 CPU)
Result: FAIL
One answer is "if it hurts, don't do it", but that's not
quite the whole story. The Travis tests use "--verbose
--tee" so that they can get the benefit of prove's parallel
options, along with a verbose log in case there is a
failure. We just need the verbose output to go to the log,
but keep stdout clean.
Getting this right turns out to be surprisingly difficult.
Here's the progression of alternatives I considered:
1. Add an option to write verbose output to stderr. This is
hard to capture, though, because we want each test to
have its own log (because they're all run in parallel
and the jumbled output would be useless).
2. Add an option to write verbose output to a file in
test-results. This works, but the log is missing all of
the non-verbose output, which gives context.
3. Like (2), but teach say_color() to additionally output
to the log. This mostly works, but misses any output
that happens outside of the say() functions (which isn't
a lot, but is a potential maintenance headache).
4. Like (2), but make the log file the same as the "--tee"
file. That almost works, but now we have two processes
opening the same file. That gives us two separate
descriptors, each with their own idea of the current
position. They'll each start writing at offset 0, and
overwrite each other's data.
5. Like (4), but in each case open the file for appending.
That atomically positions each write at the end of the
file.
It's possible we may still get sheared writes between
the two processes, but this is already the case when
writing to stdout. It's not a problem in practice
because the test harness generally waits for snippets to
finish before writing the TAP output.
We can ignore buffering issues with tee, because POSIX
mandates that it does not buffer. Likewise, POSIX
specifies "tee -a", so it should be available
everywhere.
This patch implements option (5), which seems to work well
in practice.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We are careful in test_done to handle a results directory
with a space in it, but the "--tee" code path does not.
Doing:
export TEST_OUTPUT_DIRECTORY='/tmp/path with spaces'
./t000-init.sh --tee
results in errors. Let's consistently double-quote our path
variables so that this works.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse_mailboxes should probably eventually be completely equivalent to
Mail::Address, and if this happens we can drop the Mail::Address
dependency. Add a comment in the code reminding the current state of the
code, and point to the corresponding failing test to help future
contributors to get it right.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address,
2016-10-13) improved our in-house address parser and made it closer to
Mail::Address. As a consequence, some tests comparing it to
Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The doc-tool stack does not always respect the 'tab = 8 spaces' rule,
particularly the git-scm doc pages https://git-scm.com/docs/git-merge-base
and the Git generated html pages.
Use just spaces within the block of the ascii art.
Noticed when reviewing Junio's suggested update to `git merge-base`
https://public-inbox.org/git/xmqqmvi2sj8f.fsf@gitster.mtv.corp.google.com/T/#u
Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was actually only one error message that was not yet marked for
translation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Quite a few error messages touched by this developer during the work to
speed up rebase -i started with an upper case letter, violating our
current conventions. Instead of sneaking in this fix (and forgetting
quite a few error messages), let's just have one wholesale patch fixing
all of the error messages in the sequencer.
While at it, the funny "error: Error wrapping up..." was changed to a
less funny, but more helpful, "error: failed to finalize...".
Pointed out by Junio Hamano.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes the code consistent by fixing quite a couple of error messages.
Suggested by Jakub Narębski.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The definition of this function goes back all the way to 043a449
(sequencer: factor code out of revert builtin, 2012-01-11), long before a
serious effort was made to translate all the error messages.
It is slightly out of the context of the current patch series (whose
purpose it is to re-implement the performance critical parts of the
interactive rebase in C) to make the error messages in the sequencer
translatable, but what the heck. We'll just do it while we're looking at
this part of the code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The sequencer was introduced to make the cherry-pick and revert
functionality available as library function, with the original idea
being to extend the sequencer to also implement the rebase -i
functionality.
The test to ensure that all of the commands in the script are identical
to the overall operation does not mesh well with that.
Therefore let's disable the test in rebase -i mode.
While at it, error out early if the "instruction sheet" (i.e. the todo
script) could not be parsed.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit prepares for future callers that will have a pointer/length
to some text to be written that lacks an LF, yet an LF is desired.
Instead of requiring the caller to append an LF to the buffer (and
potentially allocate memory to do so), the write_message() function
learns to append an LF at the end of the file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, we required an strbuf. But that limits the use case too much.
In the upcoming patch series (for which the current patch series prepares
the sequencer), we will want to write content to a file for which we have
a pointer and a length, not an strbuf.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no need to wait until the atexit() handler kicks in at the end.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nothing in the name "write_message()" suggests that the function
releases the strbuf passed to it. So let's release the strbuf in the
caller instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Interactive rebase's scripts may be indented; we need to handle this
case, too, now that we prepare the sequencer to process interactive
rebases.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The run_git_commit() function already knows how to amend commits, and
with this new option, it can also clean up commit messages (i.e. strip
out commented lines). This is needed to implement rebase -i's 'fixup'
and 'squash' commands as sequencer commands.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This teaches the run_git_commit() function to take an argument that will
allow us to implement "todo" commands that need to amend the commit
messages ("fixup", "squash" and "reword").
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the upcoming commits, we will implement more and more of rebase -i's
functionality inside the sequencer. One particular feature of the
commands to come is that some of them allow editing the commit message
while others don't, i.e. we cannot define in the replay_opts whether the
commit message should be edited or not.
Let's add a new parameter to the run_git_commit() function. Previously,
it was the duty of the caller to ensure that the opts->edit setting
indicates whether to let the user edit the commit message or not,
indicating that it is an "all or nothing" setting, i.e. that the
sequencer wants to let the user edit *all* commit message, or none at
all. In the upcoming rebase -i mode, it will depend on the particular
command that is currently executed, though.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As we are slowly teaching the sequencer to perform the hard work for
the interactive rebase, we need to read files that were written by
shell scripts.
These files typically contain a single line and are invariably ended
by a line feed (and possibly a carriage return before that). Let's use
a helper to read such files and to remove the line ending.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In interactive rebases, we commit a little bit differently than the
sequencer did so far: we heed the "author-script", the "message" and the
"amend" files in the .git/rebase-merge/ subdirectory.
Likewise, we may want to edit the commit message *even* when providing a
file containing the suggested commit message. Therefore we change the
code to not even provide a default message when we do not want any, and
to call the editor explicitly.
Also, in "interactive rebase" mode we want to skip reading the options
in the state directory of the cherry-pick/revert commands.
Finally, as interactive rebase's GPG settings are configured differently
from how cherry-pick (and therefore sequencer) handles them, we will
leave support for that to the next commit.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `git-rebase-todo` file contains a list of commands. Most of those
commands have the form
<verb> <sha1> <oneline>
The <oneline> is displayed primarily for the user's convenience, as
rebase -i really interprets only the <verb> <sha1> part. However, there
are *some* places in interactive rebase where the <oneline> is used to
display messages, e.g. for reporting at which commit we stopped.
So let's just remember it when parsing the todo file; we keep a copy of
the entire todo file anyway (to write out the new `done` and
`git-rebase-todo` file just before processing each command), so all we
need to do is remember the begin offsets and lengths.
As we will have to parse and remember the command-line for `exec` commands
later, we do not call the field "oneline" but rather "arg" (and will reuse
that for exec's command-line).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The subcommands are used exactly once, at the very beginning of
sequencer_pick_revisions(), to determine what to do. This is an
unnecessary level of indirection: we can simply call the correct
function to begin with. So let's do that.
While at it, ensure that the subcommands return an error code so that
they do not have to die() all over the place (bad practice for library
functions...).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is not unheard of that editors on Windows write CR/LF even if the
file originally had only LF. This is particularly awkward for exec lines
of a rebase -i todo sheet. Take for example the insn "exec echo": The
shell script parser splits at the LF and leaves the CR attached to
"echo", which leads to the unknown command "echo\r".
Work around that by stripping CR when reading the todo commands, as we
already do for LF.
This happens to fix t9903.14 and .15 in MSYS1 environments (with the
rebase--helper patches based on this patch series): the todo script
constructed in such a setup contains CR/LF thanks to MSYS1 runtime's
cleverness.
Based on a report and a patch by Johannes Sixt.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we came up with the "sequencer" idea, we really wanted to have
kind of a plumbing equivalent of the interactive rebase. Hence the
choice of words: the "todo" script, a "pick", etc.
However, when it came time to implement the entire shebang, somehow this
idea got lost and the sequencer was used as working horse for
cherry-pick and revert instead. So as not to interfere with the
interactive rebase, it even uses a separate directory to store its
state.
Furthermore, it also is stupidly strict about the "todo" script it
accepts: while it parses commands in a way that was *designed* to be
similar to the interactive rebase, it then goes on to *error out* if the
commands disagree with the overall action (cherry-pick or revert).
Finally, the sequencer code chose to deviate from the interactive rebase
code insofar that when it comes to writing the file with the remaining
commands, it *reformats* the "todo" script instead of just writing the
part of the parsed script that were not yet processed. This is not only
unnecessary churn, but might well lose information that is valuable to
the user (i.e. comments after the commands).
Let's just bite the bullet and rewrite the entire parser; the code now
becomes not only more elegant: it allows us to go on and teach the
sequencer how to parse *true* "todo" scripts as used by the interactive
rebase itself. In a way, the sequencer is about to grow up to do its
older brother's job. Better.
In particular, we choose to maintain the list of commands in an array
instead of a linked list: this is flexible enough to allow us later on to
even implement rebase -i's reordering of fixup!/squash! commits very
easily (and with a very nice speed bonus, at least on Windows).
While at it, do not stop at the first problem, but list *all* of the
problems. This will help the user when the sequencer will do `rebase
-i`'s work by allowing to address all issues in one go rather than going
back and forth until the todo list is valid.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Not only does this DRY up the code (providing a better documentation what
the code is about, as well as allowing to change the behavior in a single
place), it also makes it substantially shorter to use the same
functionality in functions to be introduced when we teach the sequencer to
process interactive-rebase's git-rebase-todo file.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
