Well, mostly. The annotation on subrepo functions tacks on a parenthetical to
the abort message, which seems reasonable for a generic mechanism. But now all
messages consistently spell out 'subrepository', and double quote the name of
the repo. I noticed the inconsistency in the change for the last commit.
As a followup to the issue4028 series, this fixes a variant of the issue
that can occur when updating with uncommited local changes.
The duplicated .hgsub warning is coming from wc.dirty(). We would previously
skip this call because it's only relevant when we're going to perform copy
tracing, which we didn't do before.
The change to the update summary line is because we now treat the rename as a
proper rename (which counts as a change), rather than an add+delete pair
(which counts as a change and a delete).
Previously, if a subrepo was added in ctx2 and then compared to another without
it (ctx1), the subrepo for ctx2 was returned amongst all of the ctx1 based
subrepos, since no subrepo exists in ctx1 to replace it in the 'subpaths' dict.
The two callers of this, basectx.status() and cmdutil.diffordiffstat(), both
compare the yielded subrepo against ctx2, and thus saw no changes when ctx2's
subrepo was returned. The tests here previously didn't mention 's/a' for the
'p1()' case.
This appears to have been a known issue, because some diffordiffstat() comments
mention that the subpath disappeared, and "the best we can do is ignore it". I
originally ran into the issue with some custom convert code to flatten a tree of
subrepos causing hg.putcommit() to abort, but this new behavior seems like the
correct status and diff behavior regardless. (The abort in convert isn't
something users will see, because convert doesn't currently support subrepos in
the official repo.)
I tried to fix this issue in the past and had to revert the fix. This is a
second attempt without the regression we found with the first one.
record defines special headers (of file) as headers whose hunk are not shown
to the user for editing, they are used to represent deleted, moved and new
files. Since we want to authorize editing the patch of newly added file we
make the newly added file with some content not special anymore. This entails
that we have to save their content before applying the backup to be able to
revert it if the patch does not apply properly.
We reintroduce the test showing that newly added files can be edited and that
their content is shown to the user.
It isn't obvious which file is the problem with deep subrepos, so provide the
path. Since the parsing is done with a ctx and not a subrepo object, it isn't
possible to display a path from the root subrepo. Therefore, the path shown is
relative to cwd.
There's no test coverage for the first abort, and I couldn't figure out how to
trigger it, but it is changed for consistency.
While using the record extension to select changes, the user couldn't see the
content of newly added files and had to select/reject them based on filename.
The test is changed accordingly in two places.
test-largefiles-update.t, test-subrepo.t, test-tag.t, and
test-rename-dir-merge.t still warn about no result returned because of
unnecessary globs that test-check-code-hg.t wants, relating to output for
pushing to, pulling from and moving X to Y.
The tests often set ui.interactive to control normally interactive prompts from
stdin. That gave an output where it was non-obvious what prompts got which
which response, and the output lacked the newline users would see after input.
Instead, if the input not is a tty, write the selection and a newline.
Before this patch, "localrepository.commit()" omits ".hgsubstate" from
"modified" (changes[0]) and "removed" (changes[2]) file list before
checking subrepositories, but leaves one in "added" (changes[1]) as it
is.
Then, "localrepository.commit()" adds ".hgsubstate" into "modified" or
"removed" list forcibly, according to subrepository statuses.
If "added" contains ".hgsubstate", the committed context will contain
two ".hgsubstate" in its "files": one from "added" (not omitted one),
and another from "modified" or "removed" (newly added one).
How many times ".hgsubstate" appears in "files" changes node hash,
even though revision content is same, because node hash calculation
uses the specified "files" directly (without duplication check or so).
This means that node hash of committed revision changes according to
existence of ".hgsubstate" in "added" at "localrepository.commit()".
".hgsubstate" is treated as "added", not only in accidental cases, but
also in the case of "qpush" for the patch adding ".hgsubstate".
This patch omits ".hgsubstate" also from "added" files before checking
subrepositories. This patch also omits ".hgsubstate" exclusion in
"qnew"/"qrefresh" introduced by changeset bbb8109a634f, because this
patch makes them meaningless.
"hg parents --template '{files}\n'" newly added to "test-mq-subrepo.t"
enhances checking unexpected multiple appearances of ".hgsubstate" in
"files" of created/refreshed MQ revisions.
Before this patch, even if specified file patterns and -I/-X options
cause listing ".hgsubstate" up in the target list, qnew/qrefresh put
".hgsubstate" into the target list individually and forcibly.
This changes how many times ".hgsubstate" appear in the target list
according to run-time conditions, and causes inconsistent node hash,
even though revision content is same, because node hash calculation
uses the specified target list directly (without duplication check or
so).
This patch always omits ".hgsubstate" from qnew/qrefresh target list
for consistent node hash.
This omitting doesn't miss including ".hgsubstate" changes, because:
- "localrepository.commit()" puts ".hgsubstate" into the target list
for "commitctx()" forcibly if needed
- "mq.putsubstate2changes()" puts ".hgsubstate" into the target list
for "patch.diff()" if it is not yet listed up
Before this patch, qnew puts updated subrepositories into target list
forcibly, if any of -I, -X or patterns are specified.
But this is meaningless and harmful, because:
- putting subrepositories into target list doesn't affect the result
of "localrepository.status()"
"dirstate.status()" invoked via "localrepository.status()" always
omits subrepositories from the result of it
- any -I/-X opts and empty "pats" causes unexpected failure
when any -I/-X opts are specified, "inclsubs" are always added to
"pats", even if "pats" is empty.
but this changes meaning of "pats" from "including all to be
included" to "including only listed subrepositories"
this may exclude ".hgsub" and cause unexpected exception raising
("can't commit subrepos without .hgsub" ).
- qnew at other than repository root (with -I, -X or any patterns)
causes unexpected failure
"scmutil.match()" treats pattern without syntax type as 'relpath'
type (= one rooted at cwd).
but qnew puts subrepository paths rooted at the repository root,
and it causes unexpected exception raising ("SUBREPO not under
root ROOT" in "pathutil.canonpath()"), if "hg qnew" is executed at
other than repository root with -I, -X or any patterns.
This patch omits meaningless and harmful putting subrepositories into
target list.
This omitting doesn't miss including updated subrepositories, because
subrepositories are specified to "scmutil.matchfiles()" directly, to
get "match" object for "localrepository.commit()".
When a subrepo has changed on the local and remote revisions, prompt the user
whether it wants to merge those subrepo revisions, keep the local revision or
keep the remote revision.
Up until now mercurial would always perform a merge on a subrepo that had
changed on the local and the remote revisions. This is often inconvenient. For
example:
- You may want to perform the actual subrepo merge after you have merged the
parent subrepo files.
- Some subrepos may be considered "read only", in the sense that you are not
supposed to add new revisions to them. In those cases "merging a subrepo" means
choosing which _existing_ revision you want to use on the merged revision. This
is often the case for subrepos that contain binary dependencies (such as DLLs,
etc).
This new prompt makes mercurial better cope with those common scenarios.
Notes:
- The default behavior (which is the one that is used when ui is not
interactive) remains unchanged (i.e. merge is the default action).
- This prompt will be shown even if the ui --tool flag is set.
- I don't know of a way to test the "keep local" and "keep remote" options (i.e.
to force the test to choose those options).
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1378420708 -7200
# Fri Sep 06 00:38:28 2013 +0200
# Node ID 2fb9cb0c7b26303ac3178b7739975e663075857d
# Parent 796d34e1b749b79834321ef1181ed8433a5515d9
merge: let the user choose to merge, keep local or keep remote subrepo revisions
When a subrepo has changed on the local and remote revisions, prompt the user
whether it wants to merge those subrepo revisions, keep the local revision or
keep the remote revision.
Up until now mercurial would always perform a merge on a subrepo that had
changed on the local and the remote revisions. This is often inconvenient. For
example:
- You may want to perform the actual subrepo merge after you have merged the
parent subrepo files.
- Some subrepos may be considered "read only", in the sense that you are not
supposed to add new revisions to them. In those cases "merging a subrepo" means
choosing which _existing_ revision you want to use on the merged revision. This
is often the case for subrepos that contain binary dependencies (such as DLLs,
etc).
This new prompt makes mercurial better cope with those common scenarios.
Notes:
- The default behavior (which is the one that is used when ui is not
interactive) remains unchanged (i.e. merge is the default action).
- This prompt will be shown even if the ui --tool flag is set.
- I don't know of a way to test the "keep local" and "keep remote" options (i.e.
to force the test to choose those options).
Up until now applying or unapplying a patch that modified .hgsubstate would not
work as expected because it would not update the subrepos according to the
.hgsubstate change. This made it very easy to lose subrepo changes when using
mq.
This revision also changes the test-mq-subrepo test so that on the qpop / qpush
tests. We no longer use the debugsub command to check the state of the subrepos
after the qpop and qpush operations. Instead we directly run the id command on
the subrepos that we want to check. The reason is that using the debugsub
command is misleading because it does not really check the state of the subrepos
on the working directory (it just returns what the change that is specified on a
given revision). Because of this the tests did not detect the problem that this
revision fixes (i.e. that applying a patch did not update the subrepos to the
corresponding revisions).
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1376350710 -7200
# Tue Aug 13 01:38:30 2013 +0200
# Node ID 60897e264858cdcd46f89e27a702086f08adca02
# Parent 2defb5453f223c3027eb2f7788fbddd52bbb3352
mq: update subrepos when applying / unapplying patches that change .hgsubstate
Up until now applying or unapplying a patch that modified .hgsubstate would not
work as expected because it would not update the subrepos according to the
.hgsubstate change. This made it very easy to lose subrepo changes when using
mq.
This revision also changes the test-mq-subrepo test so that on the qpop / qpush
tests. We no longer use the debugsub command to check the state of the subrepos
after the qpop and qpush operations. Instead we directly run the id command on
the subrepos that we want to check. The reason is that using the debugsub
command is misleading because it does not really check the state of the subrepos
on the working directory (it just returns what the change that is specified on a
given revision). Because of this the tests did not detect the problem that this
revision fixes (i.e. that applying a patch did not update the subrepos to the
corresponding revisions).
It was possible to apply, unapply, fold, patches (etc) with modified subrepos,
which resulted in surprising behavior. For example it was easy to apply a patch
with a modified subrepo, and then the refresh it and accidentally end up
including the modified subrepo on the refreshed patch.
A test has been added to verify this new check.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1375742979 -7200
# Tue Aug 06 00:49:39 2013 +0200
# Node ID a5c90acff5e61aae714ba6c9457d766c54b4f124
# Parent bacc55ce64d5de9d1eb7fcefccd3829161d5dcf7
mq: look for modified subrepos when checking for local changes
It was possible to apply, unapply, fold, patches (etc) with modified subrepos,
which resulted in surprising behavior. For example it was easy to apply a patch
with a modified subrepo, and then the refresh it and accidentally end up
including the modified subrepo on the refreshed patch.
A test has been added to verify this new check.
Before this patch, MQ checks each subrepo synchronizations against the
working directory context, so ".hgsubstate" updating is not imported
into MQ revision correctly in cases below:
- qrefresh when current ".hgsubstate" is already synchronized with
each subrepos: you can reproduce this easily by just twice or more
qrefresh invocations
- qnew just after rollback of commit which updates ".hgsubstate"
This patch resolves this by checking subrepo states against:
- the parent of "qtop" for qrefresh, or
- the parent of working context otherwise
Even though the committed revision contains diff of ".hgsubstate", the
patch file created by qrefresh doesn't contain it, because:
- ".hgsubstate" is not listed up as one of target files of the patch
for reasons below, so diff of ".hgsubstate" is not imported into
patch file
- status of ".hgsubstate" in working directory is usually "clean"
- ".hgsubstate" is not specified explicitly by users
- the patch file is created before commit processing which updates
or creates ".hgsubstate" automatically, so there is no diff for it
at that time
This patch resolves this problem by:
- putting ".hgsubstate" into target list of the patch, if needed:
this allows "patch.diff()" to import diff of ".hgsubstate" into
patch file.
- creating the patch file after commit processing:
this updates ".hgsubstate" before "patch.diff()" invocation.
For the former fixing, this patch introduces "putsubstate2changes()"
to share same implementation with qnew. This is invoked only once per
qnew/qrefresh at most, so there is less performance impact.
This patch also omits "match" argument for "patch.diff()" invocation,
because "patch.diff()" ignores "match" if "changes" is specified.
If ".hgsubstate" is already listed up as one of commit targets, qnew
put diff of ".hgsubstate" twice into the patch file stored under
".hg/patches".
It causes rejections at applying such patches.
Other than the case like in added test script, this can also occur
when qnew is executed just after rolling back the committing updated
".hgsubstate".
This patch checks whether ".hgsubstate" is already listed up as one of
commit targets, and put it into the appropriate list only if it is not
listed up yet.
Many tests didn't change back from subdirectories at the end of the tests ...
and they don't have to. The missing 'cd ..' could always be added when another
test case is added to the test file.
This change do that tests (99.5%) consistently end up in $TESTDIR where they
started, thus making it simpler to extend them or move them around.
It is possible that unrelated changes in a file are on sequential lines. The
current record extension does not allow these to be committed independently.
An example use case for this is in software development for deeply embedded
real-time systems. In these environments, it is not always possible to use a
debugger (due to time-constraints) and hence inline UART-based printing is
often used. When fixing a bug in a module, it is often convenient to add a
large number of 'printf's (linked to the UART via a custom fputc) to the module
in order to work out what is going wrong. printf is a very slow function (and
also variadic so somewhat frowned upon by the MISRA standard) and hence it is
highly undesirable to commit these lines to the repository. If only a partial
fix is implemented, however, it is desirable to commit the fix without deleting
all of the printf lines. This is also simplifies removal of the printf lines
as once the final fix is committed, 'hg revert' does the rest. It is likely
that the printf lines will be very near the actual fix, so being able to split
the hunk is very useful in this case.
There were two alternatives I considered for the user interface. One was to
manually edit the patch, the other to allow a hunk to be split into individual
lines for consideration. The latter option would require a significant
refactor of the record module and is less flexible. While the former is
potentially more complicated to use, this is a feature that is likely to only
be used in certain exceptional cases (such as the use case proposed above) and
hence I felt that the complexity would not be a considerable issue.
I've also written a follow-up patch that refactors the 'prompt' code to base
everything on the choices variable. This tidies up and clarifies the code a
bit (removes constructs like 'if ret == 7' and removes the 'e' option from the
file scope options as it's not relevant there. It's not really a necessity, so
I've excluded it from this submission for now, but I can send it separately if
there's a desire and it's on bitbucket (see below) in the meantime.
Possible future improvements include:
* Tidying up the 'prompt' code to base everything on the choices variable.
This would allow entries to be removed from the prompt as currently 'e' is
offered even for entire file patches, which is currently unsupported.
* Allowing the entire file (or even multi-file) patch to be edited manually:
this would require quite a large refactor without much benefit, so I decided
to exclude it from the initial submission.
* Allow the option to retry if a patch fails to apply (this is what Git does).
This would require quite a bit of refactoring given the current 'hg record'
implementation, so it's debatable whether it's worth it.
Output is similar to existing record user interface except that an additional
option ('e') exists to allow manual editing of the patch. This opens the
user's configured editor with the patch. A comment is added to the bottom of
the patch explaining what to do (based on Git's one).
A large proportion of the changeset is test-case changes to update the options
reported by record (Ynesfdaq? instead of Ynsfdaq?). Functional changes are in
record.py and there are some new test cases in test-record.t.
When the contents of .hgsubstate are stale (either because they've
manually been tweaked or partial updates have confused it), we get
confused about whether it actually needs committing.
So instead, we actively consult the parent's substate and compare it
the actual current state when deciding whether it needs committing.
Side effect: lots of "committing subrepo" messages that didn't
correspond with real commits disappear.
This change is fairly invasive for a fairly obscure condition, so it's
kept on the default branch.
This changeset flips the default value of ui.commitsubrepos setting
from True to False and adds a --subrepos flag to commit.
The commit, status, and diff commands behave like this with regard to
recusion and the ui.commitsubrepos setting:
| recurses | recurses
| by default | with --subrepos
--------+---------------+----------------
commit: | commitsubrepo | True
status: | False | True
diff: | False | True
By changing the default from True to False, the table becomes
consistent in the two columns:
* without --subrepos on the command line, commit will abort if a
subrepo is dirty and status/diff wont show changes inside subrepos.
* with --subrepos, all three commands will recurse.
A --subrepos flag on the command line overrides the config settin.g
- stdin was not forwarded in testrm1 and testrm2
- Forwarding content with EOL using command substitution (`foo`) does not work
correctly, the lines are joined together which breaks the prompt readline.
- EOFError is raised in ui.prompt() if the input is too short on Linux while
OSX treats it as an empty line.
This patch prevents MQ from creating an inconsistent subrepo state. If
the .hgsub file has been changed, and none of the subrepos have
uncommitted changes, creating or updating a patch (using qnew, qrefresh,
or qrecord) will update .hgsubstate accordingly.
If any subrepos _do_ have uncommitted changes, qnew/qrefresh/qrecord
will abort.
Thanks to pmezard for proposing this solution.
