| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
98ba49ccc2 (subtree: fix split processing with multiple subtrees
present, 2023-12-01) increases the performance of
git subtree split --prefix=subA
by ignoring subtree merges which are outside of `subA/`. It also
introduces a regression. Subtree merges that should be retained
are incorrectly ignored if they:
1. are nested under `subA/`; and
2. are merged with `--squash`.
For example, a subtree merged like:
git subtree merge --squash --prefix=subA/subB "$rev"
# ^^^^^^^^ ^^^^
is erroneously ignored during a split of `subA`. This causes
missing tree files and different commit hashes starting in
git v2.44.0-rc0.
The method:
should_ignore_subtree_split_commit REV
should test only a single commit REV, but the combination of
git log -1 --grep=...
actually searches all *parent* commits until a `--grep` match is
discovered.
Rewrite this method to test only one REV at a time. Extract commit
information with a single `git` call as opposed to three. The
`test` conditions for rejecting a commit remain unchanged.
Unit tests now cover nested subtrees.
Signed-off-by: Colin Stagner <ask+git@howdoi.land>
Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Allows optionally signing the commits that git subtree creates. This can
be necessary when working in a repository that requires gpg signed
commits.
Signed-off-by: Patrik Weiskircher <patrik@pspdfkit.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Optional parameter handling only works unambiguous with git rev-parse
--parseopt when using the --stuck-long option. To prepare for future commits
which add flags with optional parameters, parse with --stuck-long.
Signed-off-by: Patrik Weiskircher <patrik@pspdfkit.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Fix typos via codespell.
Signed-off-by: Andrew Kreimer <algonell@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When there are multiple subtrees present in a repository and they are
all using 'git subtree split', the 'split' command can take a
significant (and constantly growing) amount of time to run even when
using the '--rejoin' flag. This is due to the fact that when processing
commits to determine the last known split to start from when looking
for changes, if there has been a split/merge done from another subtree
there will be 2 split commits, one mainline and one subtree, for the
second subtree that are part of the processing. The non-mainline
subtree split commit will cause the processing to always need to search
the entire history of the given subtree as part of its processing even
though those commits are totally irrelevant to the current subtree
split being run.
To see this in practice you can use the open source GitHub repo
'apollo-ios-dev' and do the following in order:
-Make a changes to a file in 'apollo-ios' and 'apollo-ios-codegen'
directories
-Create a commit containing these changes
-Do a split on apollo-ios-codegen
- Do a fetch on the subtree repo
- git fetch git@github.com:apollographql/apollo-ios-codegen.git
- git subtree split --prefix=apollo-ios-codegen --squash --rejoin
- Depending on the current state of the 'apollo-ios-dev' repo
you may see the issue at this point if the last split was on
apollo-ios
-Do a split on apollo-ios
- Do a fetch on the subtree repo
- git fetch git@github.com:apollographql/apollo-ios.git
- git subtree split --prefix=apollo-ios --squash --rejoin
-Make changes to a file in apollo-ios-codegen
-Create a commit containing the change(s)
-Do a split on apollo-ios-codegen
- git subtree split --prefix=apollo-ios-codegen --squash --rejoin
-To see that the patch fixes the issue you can use the custom subtree
script in the repo so following the same steps as above, except
instead of using 'git subtree ...' for the commands use
'git-subtree.sh ...' for the commands
You will see that the final split is looking for the last split
on apollo-ios-codegen to use as it's starting point to process
commits. Since there is a split commit from apollo-ios in between the
2 splits run on apollo-ios-codegen, the processing ends up traversing
the entire history of apollo-ios which increases the time it takes to
do a split based on how long of a history apollo-ios has, while none
of these commits are relevant to the split being done on
apollo-ios-codegen.
So this commit makes a change to the processing of commits for the
split command in order to ignore non-mainline commits from other
subtrees such as apollo-ios in the above breakdown by adding a new
function 'should_ignore_subtree_commit' which is called during
'process_split_commit'. This allows the split/rejoin processing to
still function as expected but removes all of the unnecessary
processing that takes place currently which greatly inflates the
processing time. In the above example, previously the final split
would take ~10-12 minutes, while after this fix it takes seconds.
Added a test to validate that the proposed fix
solves the issue.
The test accomplishes this by checking the output
of the split command to ensure the output from
the progress of 'process_split_commit' function
that represents the 'extracount' of commits
processed remains at 0, meaning none of the commits
from the second subtree were processed.
This was tested against the original functionality
to show the test failed, and then with this fix
to show the test passes.
This illustrated that when using multiple subtrees,
A and B, when doing a split on subtree B, the
processing does not traverse the entire history
of subtree A which is unnecessary and would cause
the 'extracount' of processed commits to climb
based on the number of commits in the history of
subtree A.
Signed-off-by: Zach FettersMoore <zach.fetters@apollographql.com>
Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The `subtree_for_commit ()` helper function asserts that the subtree
identified by its parameters are either a commit or tree. This is done
via the `-o` parameter of test, which is discouraged.
Refactor the code to instead use a switch statement over the type.
Despite being aligned with our coding guidelines, the resulting code is
arguably also easier to read.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Functions in git-subtree.sh all assert that they are being passed the
correct number of arguments. In cases where we accept a variable number
of arguments we assert this via a single call to `test` with `-o`, which
is discouraged by our coding guidelines.
Convert these cases to stop doing so. This requires us to decompose
assertions of the style `assert test $# = 2 -o $# = 3` into two calls
because we have no easy way to logically chain statements passed to the
assert function.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Our coding guidelines say to not use `test` with `-a` and `-o` because
it can easily lead to bugs. Convert trivial cases where we still use
these to instead instead concatenate multiple invocations of `test` via
`&&` and `||`, respectively.
While not all of the converted instances can cause ambiguity, it is
worth getting rid of all of them regardless:
- It becomes easier to reason about the code as we do not have to
argue why one use of `-a`/`-o` is okay while another one isn't.
- We don't encourage people to use these expressions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"git subtree" only handles the negated variant of the options annotate,
prefix, onto, rejoin, ignore-joins and squash explicitly. help is
handled by "git rev-parse --parseopt" implicitly, but not its negated
form. Disable negation for it and the for the rest of the options to
get a helpful error message when trying them.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The documentation at e75d1da38a claimed support, but it was never present
Signed-off-by: Josh Soref <jsoref@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The previous commit fixed a failure in 'git subtree merge --squash' when
the previous squash-merge merged an annotated tag of the subtree
repository which is missing locally.
The same failure happens in 'git subtree split', either directly or when
called by 'git subtree push', under the same circumstances: 'cmd_split'
invokes 'find_existing_splits', which loops through previous commits and
invokes 'git rev-parse' (via 'process_subtree_split_trailer') on the
value of any 'git subtree-split' trailer it finds. This fails if this
value is the hash of an annotated tag which is missing locally.
Add a new optional argument 'repository' to 'cmd_split' and
'find_existing_splits', and invoke 'cmd_split' with that argument from
'cmd_push'. This allows 'process_subtree_split_trailer' to try to fetch
the missing tag from the 'repository' if it's not available locally,
mirroring the new behaviour of 'git subtree pull' and 'git subtree
merge'.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When 'git subtree merge --squash $ref' is invoked, either directly or
through 'git subtree pull --squash $repo $ref', the code looks for the
latest squash merge of the subtree in order to create the new merge
commit as a child of the previous squash merge.
This search is done in function 'process_subtree_split_trailer', invoked
by 'find_latest_squash', which looks for the most recent commit with a
'git-subtree-split' trailer; that trailer's value is the object name in
the subtree repository of the ref that was last squash-merged. The
function verifies that this object is present locally with 'git
rev-parse', and aborts if it's not.
The hash referenced by the 'git-subtree-split' trailer is guaranteed to
correspond to a commit since it is the result of running 'git rev-parse
-q --verify "$1^{commit}"' on the first argument of 'cmd_merge' (this
corresponds to 'rev' in 'cmd_merge' which is passed through to
'new_squash_commit' and 'squash_msg').
But this is only the case since e4f8baa88a (subtree: parse revs in
individual cmd_ functions, 2021-04-27), which went into Git 2.32. Before
that commit, 'cmd_merge' verified the revision it was given using 'git
rev-parse --revs-only "$@"'. Such an invocation, when fed the name of an
annotated tag, would return the hash of the tag, not of the commit
referenced by the tag.
This leads to a failure in 'find_latest_squash' when squash-merging if
the most recent squash-merge merged an annotated tag of the subtree
repository, using a pre-2.32 version of 'git subtree', unless that
previous annotated tag is present locally (which is not usually the
case).
We can fix this by fetching the object directly by its hash in
'process_subtree_split_trailer' when 'git rev-parse' fails, but in order
to do so we need to know the name or URL of the subtree repository.
This is not possible in general for 'git subtree merge', but is easy
when it is invoked through 'git subtree pull' since in that case the
subtree repository is passed by the user at the command line.
Allow the 'git subtree pull' scenario to work out-of-the-box by adding
an optional 'repository' argument to functions 'cmd_merge',
'find_latest_squash' and 'process_subtree_split_trailer', and invoke
'cmd_merge' with that 'repository' argument in 'cmd_pull'.
If 'repository' is absent in 'process_subtree_split_trailer', instruct
the user to try fetching the missing object directly.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Both functions 'find_latest_squash' (called by 'git subtree merge
--squash' and 'git subtree split --rejoin') and 'find_existing_splits'
(called by git 'subtree split') loop through commits that have a
'git-subtree-dir' trailer, and then process the 'git-subtree-mainline'
and 'git-subtree-split' trailers for those commits.
The processing done for the 'git-subtree-split' trailer is simple: we
check if the object exists with 'rev-parse' and set the variable
'sub' to the object name, or we die if the object does not exist.
In a future commit we will add more steps to the processing of this
trailer in order to make the code more robust.
To reduce code duplication, move the processing of the
'git-subtree-split' trailer to a dedicated function,
'process_subtree_split_trailer'.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
'cmd_pull' already checks that only two arguments are given,
'repository' and 'ref'. Define variables with these names instead of
using the positional parameter $2 and "$@".
This will allow a subsequent commit to pass 'repository' to 'cmd_merge'.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The function 'find_latest_squash' takes a single argument, 'dir', but a
debug statement uses this variable before it takes its value from $1.
This statement thus gets the value of 'dir' from the calling function,
which currently is the same as the 'dir' argument, so it works but it
is confusing.
Move the definition of 'dir' before its first use.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Just as was done in 0008d12284 (submodule: prefix die messages with
'fatal', 2021-07-10) for 'git-submodule.sh', make the 'die' messages
outputed by 'git-subtree.sh' more in line with the rest of the code base
by prefixing them with "fatal: ", and do not capitalize their first
letter.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
9a3e3ca2ba (subtree: be stricter about validating flags, 2021-04-27)
added validation code to check that options given to 'git subtree <cmd>'
made sense with the command being used.
Refactor these checks by adding a 'die_incompatible_opt' function to
reduce code duplication.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There are three occurences of 'git rev-parse <rev>' in 'git-subtree.sh'
where the command expects a revision and the script dies or exits if the
revision can't be found. In that case, the error message from 'git
rev-parse' is:
$ git rev-parse <bad rev>
<bad rev>
fatal: ambiguous argument '<bad rev>': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
This is a little confusing to the user, since this error message is
outputed by 'git subtree'.
At these points in the script, we know that we are looking for a single
revision, so be explicit by using '--verify', resulting in a little
better error message:
$ git rev-parse --verify <bad rev>
fatal: Needed a single revision
In the two occurences where we 'die' if 'git rev-parse' fails, 'git
subtree' outputs "could not rev-parse split hash $b from commit $sq", so
we actually do not need the supplementary error message from 'git
rev-parse'; add '--quiet' to silence it.
In the third occurence, we 'exit', so keep the error message from 'git
rev-parse'. Note that this messsage is still suboptimal since it can be
understood to mean that 'git rev-parse' did not receive a single
revision as argument, which is not the case here: the command did
receive a single revision, but the revision is not resolvable to an
available object.
The alternative would be to use '--' after the revision, as suggested by
the first error message, resulting in a clearer error message:
$ git rev-parse <bad rev> --
fatal: bad revision '<bad rev>'
Unfortunately we can't use that syntax because in the more common case
of the revision resolving to a known object, the command outputs the
object's hash, a newline, and the dashdash, which breaks the 'git
subtree' script.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Remove the "say" function, with various rewrites of the remaining
git-*.sh code to C and the preceding change to have git-submodule.sh
stop using the GIT_QUIET variable there were only four uses in
git-subtree.sh. Let's have it use an "arg_quiet" variable instead, and
move the "say" function over to it.
The only other use was a trivial message in git-instaweb.sh, since it
has never supported the --quiet option (or similar) that code added in
0b624b4ceee (instaweb: restart server if already running, 2009-11-22)
can simply use "echo" instead.
The remaining in-tree hits from "say" are all for the sibling function
defined in t/test-lib.sh. It's safe to remove this function since it
has never been documented in Documentation/git-sh-setup.txt.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When `merge.ff` is set to `only` in .gitconfig, `git subtree pull` will
fail with error `fatal: Not possible to fast-forward, aborting.`, but
the command does want to make merges in these places. Add `--no-ff`
argument to `git merge` to enforce this behaviour.
Signed-off-by: Thomas Koutcher <thomas.koutcher@online.fr>
Reviewed-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
315a84f9aa0 (subtree: use commits before rejoins for splits, 2018-09-28)
changed the signature of check_parents from 'check_parents [REV...]'
to 'check_parents PARENTS_EXPR INDENT'. In other words the variable list
of parent revisions became a list embedded in a string. However it
neglected to unpack the list again before sending it to cache_miss,
leading to incorrect calls whenever more than one parent was present.
This is the case whenever a merge commit is processed, with the end
result being a loss of performance from unecessary rechecks.
The indent parameter was subsequently removed in e9525a8a029 (subtree:
have $indent actually affect indentation, 2021-04-27), but the argument
handling bug remained.
For consistency, take multiple arguments in check_parents,
and pass all of them to cache_miss separately.
Signed-off-by: James Limbouris <james@digitalmatter.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
On Windows, both forward and backslash are valid separators. In
22d550749361 (subtree: don't fuss with PATH, 2021-04-27), however, we
added code that assumes that it can only be the forward slash.
Let's fix that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), `git
subtree` was broken thoroughly on Windows.
The reason is that it assumes Unix semantics, where `PATH` is
colon-separated, and it assumes that `$GIT_EXEC_PATH:` is a verbatim
prefix of `$PATH`. Neither are true, the latter in particular because
`GIT_EXEC_PATH` is a Windows-style path, while `PATH` is a Unix-style
path list.
Let's make extra certain that `$GIT_EXEC_PATH` and the first component
of `$PATH` refer to different entities before erroring out.
We do that by using the `test <path1> -ef <path2>` command that verifies
that the inode of `<path1>` and of `<path2>` is the same.
Sadly, this construct is non-portable, according to
https://pubs.opengroup.org/onlinepubs/009695399/utilities/test.html.
However, it does not matter in practice because we still first look
whether `$GIT_EXEC_PREFIX` is string-identical to the first component of
`$PATH`. This will give us the expected result everywhere but in Git for
Windows, and Git for Windows' own Bash _does_ handle the `-ef` operator.
Just in case that we _do_ need to show the error message _and_ are
running in a shell that lacks support for `-ef`, we simply suppress the
error output for that part.
This fixes https://github.com/git-for-windows/git/issues/3260
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Don't silently ignore a flag that's invalid for a given subcommand. The
user expected it to do something; we should tell the user that they are
mistaken, instead of surprising the user.
It could be argued that this change might break existing users. I'd
argue that those existing users are already broken, and they just don't
know it. Let them know that they're broken.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
'git subtree split' lets you specify a rev other than HEAD. 'git push'
lets you specify a mapping between a local thing and a remot ref. So
smash those together, and have 'git subtree push' let you specify which
local thing to run split on and push the result of that split to the
remote ref.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
'push' does a 'split' internally, but it doesn't pass flags through to the
'split'. This is silly, if you need to pass flags to 'split', then it
means that you can't use 'push'!
So, have 'push' accept 'split' flags, and pass them through to 'split'.
Add tests for this by copying split's tests with minimal modification.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Besides being a genuinely useful thing to do, this also just makes sense
and harmonizes which flags may be used when. `git subtree split
--rejoin` amounts to "automatically go ahead and do a `git subtree
merge` after doing the main `git subtree split`", so it's weird and
arbitrary that you can't pass `--squash` to `git subtree split --rejoin`
like you can `git subtree merge`. It's weird that `git subtree split
--rejoin` inherits `git subtree merge`'s `--message` but not `--squash`.
Reconcile the situation by just having `split --rejoin` actually just
call `merge` internally (or call `add` instead, as appropriate), so it
can get access to the full `merge` behavior, including `--squash`.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Just went through the docs looking for anything inaccurate or that can
be improved.
In the '-h' text, in the man page synopsis, and in the man page
description: Normalize the ordering of the list of sub-commands: 'add',
'merge', 'split', 'pull', 'push'. This allows us to kinda separate the
lower-level add/merge/split from the higher-level pull/push.
'-h' text:
- correction: Indicate that split's arg is optional.
- clarity: Emphasize that 'pull' takes the 'add'/'merge' flags.
man page:
- correction: State that all subcommands take options (it seemed to
indicate that only 'split' takes any options other than '-P').
- correction: 'split' only guarantees that the results are identical if
the flags are identical.
- correction: The flag is named '--ignore-joins', not '--ignore-join'.
- completeness: Clarify that 'push' always operates on HEAD, and that
'split' operates on HEAD if no local commit is given.
- clarity: In the description, when listing commands, repeat what their
arguments are. This way the reader doesn't need to flip back and
forth between the command description and the synopsis and the full
description to understand what's being said.
- clarity: In the <variables> used to give command arguments, give
slightly longer, descriptive names. Like <local-commit> instead of
just <commit>.
- clarity: Emphasize that 'pull' takes the 'add'/'merge' flags.
- style: In the synopsis, list options before the subcommand. This
makes things line up and be much more readable when shown
non-monospace (such as in `make html`), and also more closely matches
other man pages (like `git-submodule.txt`).
- style: Use the correct syntax for indicating the options ([<options>]
instead of [OPTIONS]).
- style: In the synopsis, separate 'pull' and 'push' from the other
lower-level commands. I think this helps readability.
- style: Code-quote things in prose that seem like they should be
code-quoted, like '.gitmodules', flags, or full commands.
- style: Minor wording improvements, like more consistent mood (many
of the command descriptions start in the imperative mood and switch
to the indicative mode by the end). That sort of thing.
- style: Capitalize "ID".
- style: Remove the "This option is only valid for XXX command" remarks
from each option, and instead rely on the section headings.
- style: Since that line is getting edited anyway, switch "behaviour" to
American "behavior".
- style: Trim trailing whitespace.
`todo`:
- style: Trim trailing whitespace.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Currently, the $indent variable is just used to track how deeply we're
nested, and the debug log is indented by things like
debug " foo"
That is: The indentation-level is hard-coded. It used to be that the
code couldn't recurse, so the indentation level could be known
statically, so it made sense to just hard-code it in the
output. However, since 315a84f9aa ("subtree: use commits before rejoins
for splits", 2018-09-28), it can now recurse, and the debug log is
misleading.
So fix that. Indent according to $indent.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Currently, debug output (triggered by passing '-d') and progress output
stomp on each other. The debug output is just streamed as lines to
stderr, and the progress output is sent to stderr as '%s\r'. When
writing to a file, it is awkward to read and difficult to distinguish
between the debug output and a progress line. When writing to a
terminal the debug lines hide progress lines.
So, when '-d' has been passed, spit out progress as 'progress: %s\n',
instead of as '%s\r', so that it can be detected, and so that the debug
lines don't overwrite the progress when written to a terminal.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
For each function in subtree, add a usage comment saying what the
arguments are, and add an `assert` checking the number of arguments.
In figuring out each thing's arguments in order to write those comments
and assertions, it turns out that find_existing_splits is written as if
it takes multiple 'revs', but it is in fact only ever passed a single
'rev':
unrevs="$(find_existing_splits "$dir" "$rev")" || exit $?
So go ahead and codify that by documenting and asserting that it takes
exactly two arguments, one dir and one rev.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
`cmd_add` starts with a check that the directory doesn't yet exist.
However, the `main` function performs the exact same check before
calling `cmd_add`. So remove the check from `cmd_add`.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The main argument parser goes ahead and tries to parse revs to make
things simpler for the sub-command implementations. But, it includes
enough special cases for different sub-commands. And it's difficult
having having to think about "is this info coming from an argument, or a
global variable?". So the main argument parser's effort to make things
"simpler" ends up just making it more confusing and complicated.
Begone with the 'revs' global variable; parse 'rev=$(...)' as needed in
individual 'cmd_*' functions.
Begone with the 'default' global variable. Its would-be value is
knowable just from which function we're in.
Begone with the 'ensure_single_rev' function. Its functionality can be
achieved by passing '--verify' to 'git rev-parse'.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
They are synonyms. Both are used in the file. ^{commit} is clearer, so
"standardize" on that.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Scripts needing to fuss with with adding $(git --exec-prefix) PATH
before loading git-sh-setup is a thing of the past. As far as I can
tell, it's been a thing of the past since since Git v1.2.0 (2006-02-12),
or more specifically, since 77cb17e940 (Exec git programs without using
PATH, 2006-01-10). However, it stuck around in contrib scripts and in
third-party scripts for long enough that it wasn't unusual to see.
Originally `git subtree` didn't fuss with PATH, but when people
(including the original subtree author) had problems, because it was a
common thing to see, it seemed that having subtree fuss with PATH was a
reasonable solution.
Here is an abridged history of fussing with PATH in subtree:
2987e6add3 (Add explicit path of git installation by 'git --exec-path', Gianluca Pacchiella, 2009-08-20)
As pointed out by documentation, the correct use of 'git-sh-setup' is
using $(git --exec-path) to avoid problems with not standard
installations.
-. git-sh-setup
+. $(git --exec-path)/git-sh-setup
33aaa697a2 (Improve patch to use git --exec-path: add to PATH instead, Avery Pennarun, 2009-08-26)
If you (like me) are using a modified git straight out of its source
directory (ie. without installing), then --exec-path isn't actually correct.
Add it to the PATH instead, so if it is correct, it'll work, but if it's
not, we fall back to the previous behaviour.
-. $(git --exec-path)/git-sh-setup
+PATH=$(git --exec-path):$PATH
+. git-sh-setup
9c632ea29c ((Hopefully) fix PATH setting for msysgit, Avery Pennarun, 2010-06-24)
Reported by Evan Shaw. The problem is that $(git --exec-path) includes a
'git' binary which is incompatible with the one in /usr/bin; if you run it,
it gives you an error about libiconv2.dll.
+OPATH=$PATH
PATH=$(git --exec-path):$PATH
. git-sh-setup
+PATH=$OPATH # apparently needed for some versions of msysgit
df2302d774 (Another fix for PATH and msysgit, Avery Pennarun, 2010-06-24)
Evan Shaw tells me the previous fix didn't work. Let's use this one
instead, which he says does work.
This fix is kind of wrong because it will run the "correct" git-sh-setup
*after* the one in /usr/bin, if there is one, which could be weird if you
have multiple versions of git installed. But it works on my Linux and his
msysgit, so it's obviously better than what we had before.
-OPATH=$PATH
-PATH=$(git --exec-path):$PATH
+PATH=$PATH:$(git --exec-path)
. git-sh-setup
-PATH=$OPATH # apparently needed for some versions of msysgit
First of all, I disagree with Gianluca's reading of the documentation:
- I haven't gone back to read what the documentation said in 2009, but
in my reading of the 2021 documentation is that it includes "$(git
--exec-path)/" in the synopsis for illustrative purposes, not to say
it's the proper way.
- After being executed by `git`, the git exec path should be the very
first entry in PATH, so it shouldn't matter.
- None of the scripts that are part of git do it that way.
But secondly, the root reason for fussing with PATH seems to be that
Avery didn't know that he needs to set GIT_EXEC_PATH if he's going to
use git from the source directory without installing.
And finally, Evan's issue is clearly just a bug in msysgit. I assume
that msysgit has since fixed the issue, and also msysgit has been
deprecated for 6 years now, so let's drop the workaround for it.
So, remove the line fussing with PATH. However, since subtree *is* in
'contrib/' and it might get installed in funny ways by users
after-the-fact, add a sanity check to the top of the script, checking
that it is installed correctly.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"$*" is for when you want to concatenate the args together,
whitespace-separated; and "$@" is for when you want them to be separate
strings.
There are several places in subtree that erroneously use $@ when
concatenating args together into an error message.
For instance, if the args are argv[1]="dead" and argv[2]="beef", then
the line
die "You must provide exactly one revision. Got: '$@'"
surely intends to call 'die' with the argument
argv[1]="You must provide exactly one revision. Got: 'dead beef'"
however, because the line used $@ instead of $*, it will actually call
'die' with the arguments
argv[1]="You must provide exactly one revision. Got: 'dead"
argv[2]="beef'"
This isn't a big deal, because 'die' concatenates its arguments together
anyway (using "$*"). But that doesn't change the fact that it was a
mistake to use $@ instead of $*, even though in the end $@ still ended
up doing the right thing.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Make it painfully obvious when reading the code which variables are
direct parsings of command line arguments.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
subtree currently defines its own `say` implementation, rather than
using git-sh-setups's implementation. Change that, don't re-invent the
wheel.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Instead of writing a slow `rev_is_descendant_of_branch $a $b` function
in shell, just use the fast `git merge-base --is-ancestor $b $a`.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Suport for Git versions older than 1.7.0 (older than February 2010) was
nice to have when git-subtree lived out-of-tree. But now that it lives
in git.git, it's not necessary to keep around. While it's technically
in contrib, with the standard 'git' packages for common systems
(including Arch Linux and macOS) including git-subtree, it seems
vanishingly likely to me that people are separately installing
git-subtree from git.git alongside an older 'git' install (although it
also seems vanishingly likely that people are still using >11 year old
git installs).
Not that there's much reason to remove it either, it's not much code,
and none of my changes depend on a newer git (to my knowledge, anyway;
I'm not actually testing against older git). I just figure it's an easy
piece of fat to trim, in the journey to making the whole thing easier to
hack on.
"Ignore space change" is probably helpful when viewing this diff.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Ensure that every $(subshell) that calls a function (as opposed to an
external executable) is followed by `|| exit $?`. Similarly, ensure that
every `cmd | while read; do ... done` loop is followed by `|| exit $?`.
Both of those constructs mean that it can miss `die` calls, and keep
running when it shouldn't.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Shove all of the loose code inside of a main() function.
This comes down to personal preference more than anything else. A
preference that I've developed over years of maintaining large Bash
scripts, but still a mere personal preference.
In this specific case, it's also moving the `set -- -h`, the `git
rev-parse --parseopt`, and the `. git-sh-setup` to be closer to all
the rest of the argument parsing, which is a readability win on its
own, IMO.
"Ignore space change" is probably helpful when viewing this diff.
Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
While looking at the inline help for git-subtree.sh, I noticed that
git subtree split --prefix=<prefix> <commit...>
was given as an option. However, it only really makes sense to provide
one revision because of the way the commits are forwarded to rev-parse
so change "<commit...>" to "<commit>" to reflect this. In addition,
check the arguments to ensure that only one rev is provided for all
subcommands that accept a commit.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Acked-by: Avery Pennarun <apenwarr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
After testing a previous patch at larger scale, a performance issue was
detected when using git show to locate parent revisions, with a single
run of the git show command taking 2 seconds or longer in a complex repo.
When the command is required tens or hundreds of times in a run of the
script, the additional wait time is unaccepatable. Replacing the command
with git rev-parse resulted in significantly increased performance, with
the command in question returning instantly.
Signed-off-by: Roger Strain <rstrain@swri.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When multiple identical parents are detected for a commit being considered
for copying, explicitly check whether one is the common merge base between
the commits. If so, the other commit can be used as the identical parent;
if not, a merge must be performed to maintain history.
In some situations two parents of a merge commit may appear to both have
identical subtree content with each other and the current commit. However,
those parents can potentially come from different commit graphs.
Previous behavior would simply select one of the identical parents to
serve as the replacement for this commit, based on the order in which they
were processed.
New behavior compares the merge base between the commits to determine if
a new merge commit is necessary to maintain history despite the identical
content.
Signed-off-by: Strain, Roger L <roger.strain@swri.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Adds recursive evaluation of parent commits which were not part of the
initial commit list when performing a split.
Split expects all relevant commits to be reachable from the target commit
but not reachable from any previous rejoins. However, a branch could be
based on a commit prior to a rejoin, then later merged back into the
current code. In this case, a parent to the commit will not be present in
the initial list of commits, trigging an "incorrect order" warning.
Previous behavior was to consider that commit to have no parent, creating
an original commit containing all subtree content. This commit is not
present in an existing subtree commit graph, changing commit hashes and
making pushing to a subtree repo impossible.
New behavior will recursively check these unexpected parent commits to
track them back to either an earlier rejoin, or a true original commit.
The generated synthetic commits will properly match previously-generated
commits, allowing successful pushing to a prior subtree repo.
Signed-off-by: Strain, Roger L <roger.strain@swri.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Changes the behavior of --ignore-joins to always consider a subtree add
commit, and ignore only splits and squashes.
The --ignore-joins option is documented to ignore prior --rejoin commits.
However, it additionally ignored subtree add commits generated when a
subtree was initially added to a repo.
Due to the logic which determines whether a commit is a mainline commit
or a subtree commit (namely, the presence or absence of content in the
subtree prefix) this causes commits before the initial add to appear to
be part of the subtree. An --ignore-joins split would therefore consider
those commits part of the subtree history and include them at the
beginning of the synthetic history, causing the resulting hashes to be
incorrect for all later commits.
Signed-off-by: Strain, Roger L <roger.strain@swri.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In a particularly complex repo, subtree split was not creating
compatible splits for pushing back to a separate repo. Addressing
one of the issues requires recursive handling of parent commits
that were not initially considered by the algorithm. This commit
makes no functional changes, but relocates the code to be called
recursively into a new method to simply comparisons of later
commits.
Signed-off-by: Strain, Roger L <roger.strain@swri.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
If log.showsignature is true (or --show-signature is passed) while
performing a `subtree add` or `subtree pull`, the command fails.
toptree_for_commit() calls `log` and passes the output to `commit-tree`.
If this output shows the GPG signature data, `commit-tree` throws a
fatal error.
This commit fixes the issue by adding --no-show-signature to `log` calls
in a few places, as well as using the more appropriate `rev-parse`
instead where possible.
Signed-off-by: Stephen R Guglielmo <srg@guglielmo.us>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We prefer a space between the function name and the parentheses, and no
space inside the parentheses.
The opening "{" should also be on the same line.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
