Commit graph

134 commits

Author SHA1 Message Date
Junio C Hamano
39f7331545 Merge branch 'pk/rebase-in-c-3-acts'
Rewrite "git rebase" in C.

* pk/rebase-in-c-3-acts:
  builtin rebase: stop if `git am` is in progress
  builtin rebase: actions require a rebase in progress
  builtin rebase: support --edit-todo and --show-current-patch
  builtin rebase: support --quit
  builtin rebase: support --abort
  builtin rebase: support --skip
  builtin rebase: support --continue
2018-11-02 11:04:54 +09:00
Junio C Hamano
b49ef560ed Merge branch 'ag/rebase-i-in-c'
Rewrite of the remaining "rebase -i" machinery in C.

* ag/rebase-i-in-c:
  rebase -i: move rebase--helper modes to rebase--interactive
  rebase -i: remove git-rebase--interactive.sh
  rebase--interactive2: rewrite the submodes of interactive rebase in C
  rebase -i: implement the main part of interactive rebase as a builtin
  rebase -i: rewrite init_basic_state() in C
  rebase -i: rewrite write_basic_state() in C
  rebase -i: rewrite the rest of init_revisions_and_shortrevisions() in C
  rebase -i: implement the logic to initialize $revisions in C
  rebase -i: remove unused modes and functions
  rebase -i: rewrite complete_action() in C
  t3404: todo list with commented-out commands only aborts
  sequencer: change the way skip_unnecessary_picks() returns its result
  sequencer: refactor append_todo_help() to write its message to a buffer
  rebase -i: rewrite checkout_onto() in C
  rebase -i: rewrite setup_reflog_action() in C
  sequencer: add a new function to silence a command, except if it fails
  rebase -i: rewrite the edit-todo functionality in C
  editor: add a function to launch the sequence editor
  rebase -i: rewrite append_todo_help() in C
  sequencer: make three functions and an enum from sequencer.c public
2018-11-02 11:04:53 +09:00
Stefan Beller
c7e5fe79b9 strbuf.h: format according to coding guidelines
The previous patch suggested the strbuf header to be the leading example
of how we would want our APIs to be documented. This may lead to some
scrutiny of that code and the coding style (which is different from the
API documentation style) and hence might be taken as an example on how
to format code as well.

So let's format strbuf.h in a way that we'd like to see:
* omit the extern keyword from function declarations
* name all parameters (usually the parameters are obvious from its type,
  but consider exceptions like
  `int strbuf_getwholeline_fd(struct strbuf *, int, int);`
* break overly long lines

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-29 11:21:05 -07:00
Pratik Karki
f95736288a builtin rebase: support --continue
This commit adds the option `--continue` which is used to resume
rebase after merge conflicts. The code tries to stay as close to
the equivalent shell scripts found in `git-legacy-rebase.sh` as
possible.

When continuing a rebase, the state variables are read from state_dir.
Some of the state variables are not actually stored there, such as
`upstream`. The shell script version simply does not set them, but for
consistency, we unset them in the builtin version.

Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-06 11:55:58 -07:00
Alban Gruin
2aed01811d editor: add a function to launch the sequence editor
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables.  launch_sequence_editor() is then added to launch
the sequence editor.

Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-10 11:56:22 -07:00
Junio C Hamano
c67de747f4 Merge branch 'en/rename-directory-detection-reboot'
Rename detection logic in "diff" family that is used in "merge" has
learned to guess when all of x/a, x/b and x/c have moved to z/a,
z/b and z/c, it is likely that x/d added in the meantime would also
want to move to z/d by taking the hint that the entire directory
'x' moved to 'z'.  A bug causing dirty files involved in a rename
to be overwritten during merge has also been fixed as part of this
work.  Incidentally, this also avoids updating a file in the
working tree after a (non-trivial) merge whose result matches what
our side originally had.

* en/rename-directory-detection-reboot: (36 commits)
  merge-recursive: fix check for skipability of working tree updates
  merge-recursive: make "Auto-merging" comment show for other merges
  merge-recursive: fix remainder of was_dirty() to use original index
  merge-recursive: fix was_tracked() to quit lying with some renamed paths
  t6046: testcases checking whether updates can be skipped in a merge
  merge-recursive: avoid triggering add_cacheinfo error with dirty mod
  merge-recursive: move more is_dirty handling to merge_content
  merge-recursive: improve add_cacheinfo error handling
  merge-recursive: avoid spurious rename/rename conflict from dir renames
  directory rename detection: new testcases showcasing a pair of bugs
  merge-recursive: fix remaining directory rename + dirty overwrite cases
  merge-recursive: fix overwriting dirty files involved in renames
  merge-recursive: avoid clobbering untracked files with directory renames
  merge-recursive: apply necessary modifications for directory renames
  merge-recursive: when comparing files, don't include trees
  merge-recursive: check for file level conflicts then get new name
  merge-recursive: add computation of collisions due to dir rename & merging
  merge-recursive: check for directory level conflicts
  merge-recursive: add get_directory_renames()
  merge-recursive: make a helper function for cleanup for handle_renames
  ...
2018-05-23 14:38:19 +09:00
Elijah Newren
f6f7755918 merge-recursive: check for file level conflicts then get new name
Before trying to apply directory renames to paths within the given
directories, we want to make sure that there aren't conflicts at the
file level either.  If there aren't any, then get the new name from
any directory renames.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-08 16:11:00 +09:00
Junio C Hamano
1ac0ce4d32 Merge branch 'ls/checkout-encoding'
The new "checkout-encoding" attribute can ask Git to convert the
contents to the specified encoding when checking out to the working
tree (and the other way around when checking in).

* ls/checkout-encoding:
  convert: add round trip check based on 'core.checkRoundtripEncoding'
  convert: add tracing for 'working-tree-encoding' attribute
  convert: check for detectable errors in UTF encodings
  convert: add 'working-tree-encoding' attribute
  utf8: add function to detect a missing UTF-16/32 BOM
  utf8: add function to detect prohibited UTF-16/32 BOM
  utf8: teach same_encoding() alternative UTF encoding names
  strbuf: add a case insensitive starts_with()
  strbuf: add xstrdup_toupper()
  strbuf: remove unnecessary NUL assignment in xstrdup_tolower()
2018-05-08 15:59:22 +09:00
Junio C Hamano
8b026edac3 Revert "Merge branch 'en/rename-directory-detection'"
This reverts commit e4bb62fa1e, reversing
changes made to 468165c1d8.

The topic appears to inflict severe regression in renaming merges,
even though the promise of it was that it would improve them.

We do not yet know which exact change in the topic was wrong, but in
the meantime, let's play it safe and revert it out of 'master'
before real Git-using projects are harmed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-11 18:07:11 +09:00
Junio C Hamano
a5bbc29994 Merge branch 'bc/object-id'
Conversion from uchar[20] to struct object_id continues.

* bc/object-id: (36 commits)
  convert: convert to struct object_id
  sha1_file: introduce a constant for max header length
  Convert lookup_replace_object to struct object_id
  sha1_file: convert read_sha1_file to struct object_id
  sha1_file: convert read_object_with_reference to object_id
  tree-walk: convert tree entry functions to object_id
  streaming: convert istream internals to struct object_id
  tree-walk: convert get_tree_entry_follow_symlinks internals to object_id
  builtin/notes: convert static functions to object_id
  builtin/fmt-merge-msg: convert remaining code to object_id
  sha1_file: convert sha1_object_info* to object_id
  Convert remaining callers of sha1_object_info_extended to object_id
  packfile: convert unpack_entry to struct object_id
  sha1_file: convert retry_bad_packed_offset to struct object_id
  sha1_file: convert assert_sha1_type to object_id
  builtin/mktree: convert to struct object_id
  streaming: convert open_istream to use struct object_id
  sha1_file: convert check_sha1_signature to struct object_id
  sha1_file: convert read_loose_object to use struct object_id
  builtin/index-pack: convert struct ref_delta_entry to object_id
  ...
2018-04-10 08:25:45 +09:00
Junio C Hamano
e4bb62fa1e Merge branch 'en/rename-directory-detection'
Rename detection logic in "diff" family that is used in "merge" has
learned to guess when all of x/a, x/b and x/c have moved to z/a,
z/b and z/c, it is likely that x/d added in the meantime would also
want to move to z/d by taking the hint that the entire directory
'x' moved to 'z'.  A bug causing dirty files involved in a rename
to be overwritten during merge has also been fixed as part of this
work.

* en/rename-directory-detection: (29 commits)
  merge-recursive: ensure we write updates for directory-renamed file
  merge-recursive: avoid spurious rename/rename conflict from dir renames
  directory rename detection: new testcases showcasing a pair of bugs
  merge-recursive: fix remaining directory rename + dirty overwrite cases
  merge-recursive: fix overwriting dirty files involved in renames
  merge-recursive: avoid clobbering untracked files with directory renames
  merge-recursive: apply necessary modifications for directory renames
  merge-recursive: when comparing files, don't include trees
  merge-recursive: check for file level conflicts then get new name
  merge-recursive: add computation of collisions due to dir rename & merging
  merge-recursive: check for directory level conflicts
  merge-recursive: add get_directory_renames()
  merge-recursive: make a helper function for cleanup for handle_renames
  merge-recursive: split out code for determining diff_filepairs
  merge-recursive: make !o->detect_rename codepath more obvious
  merge-recursive: fix leaks of allocated renames and diff_filepairs
  merge-recursive: introduce new functions to handle rename logic
  merge-recursive: move the get_renames() function
  directory rename detection: tests for handling overwriting dirty files
  directory rename detection: tests for handling overwriting untracked files
  ...
2018-04-10 08:25:43 +09:00
brian m. carlson
30e677e0e2 strbuf: convert strbuf_add_unique_abbrev to use struct object_id
Convert the declaration and definition of strbuf_add_unique_abbrev to
make it take a pointer to struct object_id.  Predeclare the struct in
strbuf.h, as cache.h includes strbuf.h before it declares the struct,
and otherwise the struct declaration would have the wrong scope.

Apply the following semantic patch, along with the standard object_id
transforms, to adjust the callers:

@@
expression E1, E2, E3;
@@
- strbuf_add_unique_abbrev(E1, E2.hash, E3);
+ strbuf_add_unique_abbrev(E1, &E2, E3);

@@
expression E1, E2, E3;
@@
- strbuf_add_unique_abbrev(E1, E2->hash, E3);
+ strbuf_add_unique_abbrev(E1, E2, E3);

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14 09:23:48 -07:00
Lars Schneider
13ecb4638e strbuf: add xstrdup_toupper()
Create a copy of an existing string and make all characters upper case.
Similar xstrdup_tolower().

This function is used in a subsequent commit.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-15 11:36:15 -08:00
Elijah Newren
79d49b7d8c merge-recursive: check for file level conflicts then get new name
Before trying to apply directory renames to paths within the given
directories, we want to make sure that there aren't conflicts at the
file level either.  If there aren't any, then get the new name from
any directory renames.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-14 13:02:53 -08:00
Nguyễn Thái Ngọc Duy
c64a8d200f worktree move: accept destination as directory
Similar to "mv a b/", which is actually "mv a b/a", we extract basename
of source worktree and create a directory of the same name at
destination if dst path is a directory.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-12 13:13:35 -08:00
Elijah Newren
9881f21190 strbuf: remove unused stripspace function alias
In commit 63af4a8446 ("strbuf: make stripspace() part of strbuf",
2015-10-16), stripspace() was moved to strbuf and renamed to
strbuf_stripspace().  A "temporary" alias was added for the old name until
all topic branches had time to switch over.  They have had time, so remove
the old alias.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-05 08:50:15 -08:00
Junio C Hamano
aae4788eee Merge branch 'jn/strbuf-doc-re-reuse'
* jn/strbuf-doc-re-reuse:
  strbuf doc: reuse after strbuf_release is fine
2017-10-07 16:27:53 +09:00
Jonathan Nieder
e0222159fa strbuf doc: reuse after strbuf_release is fine
strbuf_release leaves the strbuf in a valid, initialized state, so
there is no need to call strbuf_init after it.

Moreover, this is not likely to change in the future: strbuf_release
leaving the strbuf in a valid state has been easy to maintain and has
been very helpful for Git's robustness and simplicity (e.g.,
preventing use-after-free vulnerabilities).

Document the semantics so the next generation of Git developers can
become familiar with them without reading the implementation.  It is
still not advisable to call strbuf_release too often because it is
wasteful, so add a note pointing to strbuf_reset for that.

The same semantics apply to strbuf_detach.  Add a similar note to its
docstring to make that clear.

Improved-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-04 15:21:52 +09:00
Junio C Hamano
a48ce37858 Merge branch 'ma/ts-cleanups'
Assorted bugfixes and clean-ups.

* ma/ts-cleanups:
  ThreadSanitizer: add suppressions
  strbuf_setlen: don't write to strbuf_slopbuf
  pack-objects: take lock before accessing `remaining`
  convert: always initialize attr_action in convert_attrs
2017-09-10 17:08:22 +09:00
Martin Ågren
65961d5a75 strbuf_setlen: don't write to strbuf_slopbuf
strbuf_setlen(., 0) writes '\0' to sb.buf[0], where buf is either
allocated and unique to sb, or the global slopbuf. The slopbuf is meant
to provide a guarantee that buf is not NULL and that a freshly
initialized buffer contains the empty string, but it is not supposed to
be written to. That strbuf_setlen writes to slopbuf has at least two
implications:

First, it's wrong in principle. Second, it might be hiding misuses which
are just waiting to wreak havoc. Third, ThreadSanitizer detects a race
when multiple threads write to slopbuf at roughly the same time, thus
potentially making any more critical races harder to spot.

Avoid writing to strbuf_slopbuf in strbuf_setlen. Let's instead assert
on the first byte of slopbuf being '\0', since it helps ensure the
promised invariant of buf[len] == '\0'. (We know that "len" was already
0, or someone has messed with "alloc". If someone has fiddled with the
fields that much beyond the correct interface, they're on their own.)

This is a function which is used in many places, possibly also in hot
code paths. There are two branches in strbuf_setlen already, and we are
adding a third and possibly a fourth (in the assert). In hot code paths,
we hopefully reuse the buffer in order to avoid continous reallocations.
Thus, after a start-up phase, we should always take the same path,
which might help branch prediction, and we would never make the assert.
If a hot code path continuously reallocates, we probably have bigger
performance problems than this new safety-check.

Simple measurements do not contradict this reasoning. 100000000 times
resetting a buffer and adding the empty string takes 5.29/5.26 seconds
with/without this patch (best of three). Releasing at every iteration
yields 18.01/17.87. Adding a 30-character string instead of the empty
string yields 5.61/5.58 and 17.28/17.28(!).

This patch causes the git binary emitted by gcc 5.4.0 -O2 on my machine
to grow from 11389848 bytes to 11497184 bytes, an increase of 0.9%.

I also tried to piggy-back on the fact that we already check alloc,
which should already tell us whether we are using the slopbuf:

        if (sb->alloc) {
                if (len > sb->alloc - 1)
                        die("BUG: strbuf_setlen() beyond buffer");
                sb->buf[len] = '\0';
        } else {
                if (len)
                        die("BUG: strbuf_setlen() beyond buffer");
                assert(!strbuf_slopbuf[0]);
        }
        sb->len = len;

That didn't seem to be much slower (5.38, 18.02, 5.70, 17.32 seconds),
but it does introduce some minor code duplication. The resulting git
binary was 11510528 bytes large (another 0.1% increase).

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-23 10:38:46 -07:00
Jeff King
cbc0f81d96 strbuf: use designated initializers in STRBUF_INIT
There are certain C99 features that might be nice to use in
our code base, but we've hesitated to do so in order to
avoid breaking compatibility with older compilers. But we
don't actually know if people are even using pre-C99
compilers these days.

One way to figure that out is to introduce a very small use
of a feature, and see if anybody complains. The strbuf code
is a good place to do this for a few reasons:

  - it always gets compiled, no matter which Makefile knobs
    have been tweaked.

  - it's very stable; this definition hasn't changed in a
    long time and is not likely to (so if we have to revert,
    it's unlikely to cause headaches)

If this patch can survive a few releases without complaint,
then we can feel more confident that designated initializers
are widely supported by our user base.  It also is an
indication that other C99 features may be supported, but not
a guarantee (e.g., gcc had designated initializers before
C99 existed).

And if we do get complaints, then we'll have gained some
data and we can easily revert this patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-14 08:32:44 -07:00
Junio C Hamano
6ba649e408 Merge branch 'ab/strbuf-addftime-tzname-boolify'
strbuf_addftime() is further getting tweaked.

* ab/strbuf-addftime-tzname-boolify:
  strbuf: change an always NULL/"" strbuf_addftime() param to bool
  strbuf.h comment: discuss strbuf_addftime() arguments in order
2017-07-06 18:14:47 -07:00
Ævar Arnfjörð Bjarmason
3b702239d6 strbuf: change an always NULL/"" strbuf_addftime() param to bool
strbuf_addftime() allows callers to pass a time zone name for
expanding %Z. The only current caller either passes the empty string
or NULL, in which case %Z is handed over verbatim to strftime(3).
Replace that string parameter with a flag controlling whether to
remove %Z from the format specification. This simplifies the code.

Commit-message-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-01 10:47:05 -07:00
Junio C Hamano
8af3c643d9 Merge branch 'rs/pretty-add-again'
The pretty-format specifiers like '%h', '%t', etc. had an
optimization that no longer works correctly.  In preparation/hope
of getting it correctly implemented, first discard the optimization
that is broken.

* rs/pretty-add-again:
  pretty: recalculate duplicate short hashes
2017-06-24 14:28:38 -07:00
Ævar Arnfjörð Bjarmason
4904cbc9e1 strbuf.h comment: discuss strbuf_addftime() arguments in order
Change the comment documenting the strbuf_addftime() function to
discuss the parameters in the order in which they appear, which makes
this easier to read than discussing them out of order.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-24 11:15:59 -07:00
René Scharfe
c3fbf81a85 strbuf: let strbuf_addftime handle %z and %Z itself
There is no portable way to pass timezone information to strftime.  Add
parameters for timezone offset and name to strbuf_addftime and let it
handle the timezone-related format specifiers %z and %Z internally.

Callers can opt out for %Z by passing NULL as timezone name.  %z is
always handled internally -- this helps on Windows, where strftime would
expand it to a timezone name (same as %Z), in violation of POSIX.
Modifiers are not handled, e.g. %Ez is still passed to strftime.

Use an empty string as timezone name in show_date (the only current
caller) for now because we only have the timezone offset in non-local
mode.  POSIX allows %Z to resolve to an empty string in case of missing
information.

Helped-by: Ulrich Mueller <ulm@gentoo.org>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 14:34:37 -07:00
René Scharfe
fe9e2aefd4 pretty: recalculate duplicate short hashes
b9c6232138 (--format=pretty: avoid calculating expensive expansions
twice) optimized adding short hashes multiple times by using the
fact that the output strbuf was only ever simply appended to and
copying the added string from the previous run.  That prerequisite
is no longer given; we now have modfiers like %< and %+ that can
cause the cache to lose track of the correct offsets.  Remove it.

Reported-by: Michael Giuffrida <michaelpg@chromium.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 11:40:53 -07:00
Junio C Hamano
c809496c97 Merge branch 'jk/interpret-branch-name'
"git branch @" created refs/heads/@ as a branch, and in general the
code that handled @{-1} and @{upstream} was a bit too loose in
disambiguating.

* jk/interpret-branch-name:
  checkout: restrict @-expansions when finding branch
  strbuf_check_ref_format(): expand only local branches
  branch: restrict @-expansions when deleting
  t3204: test git-branch @-expansion corner cases
  interpret_branch_name: allow callers to restrict expansions
  strbuf_branchname: add docstring
  strbuf_branchname: drop return value
  interpret_branch_name: move docstring to header file
  interpret_branch_name(): handle auto-namelen for @{-1}
2017-03-14 15:23:18 -07:00
Junio C Hamano
fc32293502 Merge branch 'rs/strbuf-add-real-path'
An helper function to make it easier to append the result from
real_path() to a strbuf has been added.

* rs/strbuf-add-real-path:
  strbuf: add strbuf_add_real_path()
  cocci: use ALLOC_ARRAY
2017-03-10 13:24:23 -08:00
Jeff King
0e9f62dab9 interpret_branch_name: allow callers to restrict expansions
The interpret_branch_name() function converts names like
@{-1} and @{upstream} into branch names. The expanded ref
names are not fully qualified, and may be outside of the
refs/heads/ namespace (e.g., "@" expands to "HEAD", and
"@{upstream}" is likely to be in "refs/remotes/").

This is OK for callers like dwim_ref() which are primarily
interested in resolving the resulting name, no matter where
it is. But callers like "git branch" treat the result as a
branch name in refs/heads/.  When we expand to a ref outside
that namespace, the results are very confusing (e.g., "git
branch @" tries to create refs/heads/HEAD, which is
nonsense).

Callers can't know from the returned string how the
expansion happened (e.g., did the user really ask for a
branch named "HEAD", or did we do a bogus expansion?). One
fix would be to return some out-parameters describing the
types of expansion that occurred. This has the benefit that
the caller can generate precise error messages ("I
understood @{upstream} to mean origin/master, but that is a
remote tracking branch, so you cannot create it as a local
name").

However, out-parameters make the function interface somewhat
cumbersome. Instead, let's do the opposite: let the caller
tell us which elements to expand. That's easier to pass in,
and none of the callers give more precise error messages
than "@{upstream} isn't a valid branch name" anyway (which
should be sufficient).

The strbuf_branchname() function needs a similar parameter,
as most of the callers access interpret_branch_name()
through it.

We can break the callers down into two groups:

  1. Callers that are happy with any kind of ref in the
     result. We pass "0" here, so they continue to work
     without restrictions. This includes merge_name(),
     the reflog handling in add_pending_object_with_path(),
     and substitute_branch_name(). This last is what powers
     dwim_ref().

  2. Callers that have funny corner cases (mostly in
     git-branch and git-checkout). These need to make use of
     the new parameter, but I've left them as "0" in this
     patch, and will address them individually in follow-on
     patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
Jeff King
0705fe202d strbuf_branchname: add docstring
This function and its companion, strbuf_check_branch_ref(),
did not have their purpose or semantics explained. Let's do
so.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
Jeff King
311fc74826 strbuf_branchname: drop return value
The return value from strbuf_branchname() is confusing and
useless: it's 0 if the whole name was consumed by an @-mark,
but otherwise is the length of the original name we fed.

No callers actually look at the return value, so let's just
get rid of it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:05:04 -08:00
René Scharfe
33ad9ddd0b strbuf: add strbuf_add_real_path()
Add a function for appending the canonized absolute pathname of a given
path to a strbuf.  It keeps the existing contents intact, as expected of
a function of the strbuf_add() family, while avoiding copying the result
if the given strbuf is empty.  It's more consistent with the rest of the
strbuf API than strbuf_realpath(), which it's wrapping.

Also add a semantic patch demonstrating its intended usage and apply it
to the current tree.  Using strbuf_add_real_path() instead of calling
strbuf_addstr() and real_path() avoids an extra copy to a static buffer.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-27 11:02:06 -08:00
René Scharfe
35d803bc9a use SWAP macro
Apply the semantic patch swap.cocci to convert hand-rolled swaps to use
the macro SWAP.  The resulting code is shorter and easier to read, the
object code is effectively unchanged.

The patch for object.c had to be hand-edited in order to preserve the
comment before the change; Coccinelle tried to eat it for some reason.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-30 14:17:00 -08:00
Jeff King
670c359da3 link_alt_odb_entry: handle normalize_path errors
When we add a new alternate to the list, we try to normalize
out any redundant "..", etc. However, we do not look at the
return value of normalize_path_copy(), and will happily
continue with a path that could not be normalized. Worse,
the normalizing process is done in-place, so we are left
with whatever half-finished working state the normalizing
function was in.

Fortunately, this cannot cause us to read past the end of
our buffer, as that working state will always leave the
NUL from the original path in place. And we do tend to
notice problems when we check is_directory() on the path.
But you can see the nonsense that we feed to is_directory
with an entry like:

  this/../../is/../../way/../../too/../../deep/../../to/../../resolve

in your objects/info/alternates, which yields:

  error: object directory
  /to/e/deep/too/way//ects/this/../../is/../../way/../../too/../../deep/../../to/../../resolve
  does not exist; check .git/objects/info/alternates.

We can easily fix this just by checking the return value.
But that makes it hard to generate a good error message,
since we're normalizing in-place and our input value has
been overwritten by cruft.

Instead, let's provide a strbuf helper that does an in-place
normalize, but restores the original contents on error. This
uses a second buffer under the hood, which is slightly less
efficient, but this is not a performance-critical code path.

The strbuf helper can also properly set the "len" parameter
of the strbuf before returning. Just doing:

  normalize_path_copy(buf.buf, buf.buf);

will shorten the string, but leave buf.len at the original
length. That may be confusing to later code which uses the
strbuf.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10 13:52:36 -07:00
Junio C Hamano
48aa37ed42 Merge branch 'rs/use-strbuf-addbuf' into maint
Code cleanup.

* rs/use-strbuf-addbuf:
  strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
  use strbuf_addbuf() for appending a strbuf to another
2016-08-08 14:21:42 -07:00
Junio C Hamano
b4e8a847ba Merge branch 'rs/use-strbuf-addbuf'
Code cleanup.

* rs/use-strbuf-addbuf:
  strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
  use strbuf_addbuf() for appending a strbuf to another
2016-07-25 14:13:47 -07:00
René Scharfe
31471ba21e strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()
Implement strbuf_addbuf() as a normal function in order to avoid calling
strbuf_grow() twice, with the second callinside strbud_add() being a
no-op.  This is slightly faster and also reduces the text size a bit.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-22 09:22:26 -07:00
Junio C Hamano
f7927316cf Merge branch 'pb/strbuf-read-file-doc' into maint
Minor doc update.

* pb/strbuf-read-file-doc:
  strbuf: describe the return value of strbuf_read_file
2016-07-06 13:06:45 -07:00
Junio C Hamano
ed319fca33 Merge branch 'pb/strbuf-read-file-doc'
* pb/strbuf-read-file-doc:
  strbuf: describe the return value of strbuf_read_file
2016-06-27 09:56:46 -07:00
Pranit Bauva
ed008d7bb9 strbuf: describe the return value of strbuf_read_file
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-14 10:57:21 -07:00
Junio C Hamano
bdebbeb334 Merge branch 'sb/submodule-parallel-update'
A major part of "git submodule update" has been ported to C to take
advantage of the recently added framework to run download tasks in
parallel.

* sb/submodule-parallel-update:
  clone: allow an explicit argument for parallel submodule clones
  submodule update: expose parallelism to the user
  submodule helper: remove double 'fatal: ' prefix
  git submodule update: have a dedicated helper for cloning
  run_processes_parallel: rename parameters for the callbacks
  run_processes_parallel: treat output of children as byte array
  submodule update: direct error message to stderr
  fetching submodules: respect `submodule.fetchJobs` config option
  submodule-config: drop check against NULL
  submodule-config: keep update strategy around
2016-04-06 11:39:01 -07:00
Stefan Beller
2dac9b5637 run_processes_parallel: treat output of children as byte array
We do not want the output to be interrupted by a NUL byte, so we
cannot use raw fputs. Introduce strbuf_write to avoid having long
arguments in run-command.c.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-01 11:57:19 -08:00
Junio C Hamano
b62624b51a Merge branch 'jc/strbuf-getline'
The preliminary clean-up for jc/peace-with-crlf topic.

* jc/strbuf-getline:
  strbuf: give strbuf_getline() to the "most text friendly" variant
  checkout-index: there are only two possible line terminations
  update-index: there are only two possible line terminations
  check-ignore: there are only two possible line terminations
  check-attr: there are only two possible line terminations
  mktree: there are only two possible line terminations
  strbuf: introduce strbuf_getline_{lf,nul}()
  strbuf: make strbuf_getline_crlf() global
  strbuf: miniscule style fix
2016-01-28 16:10:14 -08:00
Junio C Hamano
1a0c8dfd89 strbuf: give strbuf_getline() to the "most text friendly" variant
Now there is no direct caller to strbuf_getline(), we can demote it
to file-scope static that is private to strbuf.c and rename it to
strbuf_getdelim().  Rename strbuf_getline_crlf(), which is designed
to be the most "text friendly" variant, and allow it to take over
this simplest name, strbuf_getline(), so we can add more uses of it
without having to type _crlf over and over again in the coming
steps.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15 10:23:57 -08:00
Junio C Hamano
8f309aeb82 strbuf: introduce strbuf_getline_{lf,nul}()
The strbuf_getline() interface allows a byte other than LF or NUL as
the line terminator, but this is only because I wrote these
codepaths anticipating that there might be a value other than NUL
and LF that could be useful when I introduced line_termination long
time ago.  No useful caller that uses other value has emerged.

By now, it is clear that the interface is overly broad without a
good reason.  Many codepaths have hardcoded preference to read
either LF terminated or NUL terminated records from their input, and
then call strbuf_getline() with LF or NUL as the third parameter.

This step introduces two thin wrappers around strbuf_getline(),
namely, strbuf_getline_lf() and strbuf_getline_nul(), and
mechanically rewrites these call sites to call either one of
them.  The changes contained in this patch are:

 * introduction of these two functions in strbuf.[ch]

 * mechanical conversion of all callers to strbuf_getline() with
   either '\n' or '\0' as the third parameter to instead call the
   respective thin wrapper.

After this step, output from "git grep 'strbuf_getline('" would
become a lot smaller.  An interim goal of this series is to make
this an empty set, so that we can have strbuf_getline_crlf() take
over the shorter name strbuf_getline().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15 10:12:51 -08:00
Junio C Hamano
c8aa9fdf5d strbuf: make strbuf_getline_crlf() global
Often we read "text" files that are supplied by the end user
(e.g. commit log message that was edited with $GIT_EDITOR upon 'git
commit -e'), and in some environments lines in a text file are
terminated with CRLF.  Existing strbuf_getline() knows to read a
single line and then strip the terminating byte from the result, but
it is handy to have a version that is more tailored for a "text"
input that takes both '\n' and '\r\n' as line terminator (aka
<newline> in POSIX lingo) and returns the body of the line after
stripping <newline>.

Recently reimplemented "git am" uses such a function implemented
privately; move it to strbuf.[ch] and make it available for others.

Note that we do not blindly replace calls to strbuf_getline() that
uses LF as the line terminator with calls to strbuf_getline_crlf()
and this is very much deliberate.  Some callers may want to treat an
incoming line that ends with CR (and terminated with LF) to have a
payload that includes the final CR, and such a blind replacement
will result in misconversion when done without code audit.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-14 15:05:55 -08:00
Stefan Beller
b4e04fb66e strbuf: add strbuf_read_once to read without blocking
The new call will read from a file descriptor into a strbuf once. The
underlying call xread is just run once. xread only reattempts
reading in case of EINTR, which makes it suitable to use for a
nonblocking read.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-16 12:06:08 -08:00
Junio C Hamano
1ad7c0f689 Merge branch 'tk/stripspace'
The internal stripspace() function has been moved to where it
logically belongs to, i.e. strbuf API, and the command line parser
of "git stripspace" has been updated to use the parse_options API.

* tk/stripspace:
  stripspace: use parse-options for command-line parsing
  strbuf: make stripspace() part of strbuf
2015-10-26 15:55:20 -07:00
Tobias Klauser
63af4a8446 strbuf: make stripspace() part of strbuf
This function is also used in other builtins than stripspace, so it
makes sense to have it in a more generic place.  Since it operates
on an strbuf and the function is declared in strbuf.h, move it to
strbuf.c and add the corresponding prefix to its name, just like
other API functions in the strbuf_* family.

Also switch all current users of stripspace() to the new function
name and keep a temporary wrapper inline function for any topic
branches still using stripspace().

Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-16 09:45:15 -07:00