D1144: directaccess: add support to export and tests to demonstrate things
pulkit updated this revision to Diff 3206.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D1144?vs=2907&id=3206
REVISION DETAIL
https://phab.mercurial-scm.org/D1144
AFFECTED FILES
mercurial/commands.py
tests/test-directaccess.t
CHANGE DETAILS
diff --git a/tests/test-directaccess.t b/tests/test-directaccess.t
new file mode 100644
--- /dev/null
+++ b/tests/test-directaccess.t
@@ -0,0 +1,68 @@
+Tests for access level on hidden commits by various commands on based of their
+type.
+
+Setting the required config to start this
+
+ $ cat >> $HGRCPATH < [experimental]
+ > evolution=createmarkers, allowunstable
+ > directaccess=True
+ > [extensions]
+ > amend =
+ > EOF
+
+ $ hg init repo
+ $ cd repo
+ $ for ch in {a..c}; do touch $ch; echo "foo" >> $ch; hg ci -Aqm "Added "$ch;
done
+
+ $ hg log -G -T '{rev}:{node} {desc}' --hidden
+ @ 2:28ad74487de9599d00d81085be739c61fc340652 Added c
+ |
+ o 1:29becc82797a4bc11ec8880b58eaecd2ab3e7760 Added b
+ |
+ o 0:18d04c59bb5d2d4090ad9a5b59bd6274adb63add Added a
+
+ $ echo "bar" >> c
+ $ hg amend
+
+ $ hg log -G -T '{rev}:{node} {desc}' --hidden
+ @ 3:2443a0e664694756d8b435d06b6ad84f941b6fc0 Added c
+ |
+ | x 2:28ad74487de9599d00d81085be739c61fc340652 Added c
+ |/
+ o 1:29becc82797a4bc11ec8880b58eaecd2ab3e7760 Added b
+ |
+ o 0:18d04c59bb5d2d4090ad9a5b59bd6274adb63add Added a
+
+Testing read only commands on the hidden revision
+
+Testing with rev number
+
+ $ hg exp 2
+ abort: hidden revision '2'!
+ (use --hidden to access hidden revisions)
+ [255]
+
+Testing with hash
+
+ $ hg exp 28ad74
+ # HG changeset patch
+ # User test
+ # Date 0 0
+ # Thu Jan 01 00:00:00 1970 +
+ # Node ID 28ad74487de9599d00d81085be739c61fc340652
+ # Parent 29becc82797a4bc11ec8880b58eaecd2ab3e7760
+ Added c
+
+ diff -r 29becc82797a -r 28ad74487de9 c
+ --- /dev/nullThu Jan 01 00:00:00 1970 +
+ +++ b/c Thu Jan 01 00:00:00 1970 +
+ @@ -0,0 +1,1 @@
+ +foo
+
+Make sure things with cmdtype undefined dont work with hidden commits
+
+ $ hg log -r 28ad74
+ abort: hidden revision '28ad74'!
+ (use --hidden to access hidden revisions)
+ [255]
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -61,6 +61,8 @@
release = lockmod.release
+readonly = registrar.command.readonly
+
table = {}
table.update(debugcommandsmod.command._table)
@@ -1863,7 +1865,8 @@
('', 'switch-parent', None, _('diff against the second parent')),
('r', 'rev', [], _('revisions to export'), _('REV')),
] + diffopts,
-_('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'))
+_('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'),
+cmdtype=readonly)
def export(ui, repo, *changesets, **opts):
"""dump the header and diffs for one or more changesets
To: pulkit, #hg-reviewers, lothiraldan
Cc: durham, dlax, lothiraldan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1144: directaccess: add support to export and tests to demonstrate things
durham added a comment. @lothiraldan Are pinned revs deleted when the cache is rebuilt? I would kinda not expect them to be. If they are deleted, then yea moving these exceptions to another attribute that is persisted across cache clears makes sense. For your concern about changing from a command-level option to a scope and revrange level option, I have a couple comments: - If we went the current command-level flag, the examples your talking about where it's initially ambiguous if the command is actually reading or writing the hidden commit (phase, fold --reuse-msg, etc) would default to warn mode and the only negative effect would be the user gets an extra warning message telling them a hash they passed is hidden. Probably not a common occurrence, and not a huge issue. In the future we could add more logic within the command to suppress that warning when doing the read only paths, but for the short term this let's us get directaccess out the door without auditing and updating every commands logic. - As for adding exception context manager type things, I think one of the issues with our current visibility code is that it requires the person writing a command to be too aware that visibility exists. If we start requiring commands to use this new context appropriately, and to pass the right argument to revrange at the right time, I think we just introduce more risk of commands being written incorrectly and having more complexity at the business logic layer. If we can avoid all of that at the cost of printing warnings to the user slightly more often, I think that's a good trade off. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1144 To: pulkit, #hg-reviewers, lothiraldan Cc: durham, dlax, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1144: directaccess: add support to export and tests to demonstrate things
lothiraldan requested changes to this revision. lothiraldan added a comment. This revision now requires changes to proceed. I'm +1 on the feature. But I have several concerns with the current implementation: - Marking a whole command as readonly or not is too simplistic I'm afraid, we have commands (like phase) that can be readonly or not based on their parameters. I think we need a more fine-grained selection and that commands needs to pass the desired directaccess mode to `scmutil.revrange`. For example, imagine we add a `--reuse-message REV` argument to fold, that to select a revision to use the message from. The usual argument of fold rewrite the changes (so direct access is off). However having a more relax direct access for the --reuse-message make sense. - Putting direct access hashes in pinnedrevs attribute while having the same filter function for all filters could lead to have visible filter excludind the direct accessed hashes when we bust the volatile sets which will require a full cache bust at the next check. Instead, I propose: - To clarify the vocabulary: direct access allow for node explicitly specified to be made visible for the duration of a command. We could call such revisions `visibility exceptions`. To achieve this the `exceptions` need to be excluded from the usual filter applied to the repository during a command. - Put the direct access hashes, not in pinnedrevs but in a separate attribute, maybe with a dedicated API `repo.addvisibilityexception(revs)` that could also be used by rebase and histedit as they already have some exception mechanism. - Keep the new filter name, but with a dedicated filter function (`computehiddenexceptions` for example). The new dedicated function could use the attribute updated by `repo.addvisibilityexception(revs)` to exclude them from the result without ever impacting the `visible` filter. - Makes `scmutil.revrange` accepts a new parameter `visibility_exception_mode` for example which could have 3 values and could be used to replace this condition `if repo and repo.filtername in ['visible-hidden', 'visible-warnhidden']:`: - None, current behavior - `warning`, warns when directly accessed hashes are detected, could be used to replace this condition `if repo.filtername == 'visible-warnhidden':` - `silent`, unhide directly accessed hashed without warning - Now that we can distinguish between warning mode or silent mode, we could use only a single filter, `visible+exceptions` for example, and limit the scope of it with a context manager so consumers could get revisions with exceptions without needing to reset the filter by hand, I have something like this in mind: def my_command(ui, repo, *revs, foo=[]): with repo.allowexception() as repo: ... foorevs = scmutil.revrange(repo, foo, allowexception='warn') ... targetrevs = scmutil.revrange(repo, foo) ... I don't think it's the best API right now, but being able to limit explicitly the "scope" of the new filter would be beneficial for avoiding hard to debug issues and performances regressions. I couldn't find a better API than the context manager + new revrange parameter, if you have ideas please share. This proposal is focused on removing the implicit global state carried by the filter name and pinnedrevs and make the filter change scope more controllable and explicit. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1144 To: pulkit, #hg-reviewers, lothiraldan Cc: lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1144: directaccess: add support to export and tests to demonstrate things
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
This patch adds the cmdtype attribute to export command which is a read only
command and adds test for accessing hidden commits using the same command.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1144
AFFECTED FILES
mercurial/commands.py
tests/test-directaccess.t
CHANGE DETAILS
diff --git a/tests/test-directaccess.t b/tests/test-directaccess.t
new file mode 100644
--- /dev/null
+++ b/tests/test-directaccess.t
@@ -0,0 +1,68 @@
+Tests for access level on hidden commits by various commands on based of their
+type.
+
+Setting the required config to start this
+
+ $ cat >> $HGRCPATH < [experimental]
+ > evolution=createmarkers, allowunstable
+ > directaccess=True
+ > [extensions]
+ > amend =
+ > EOF
+
+ $ hg init repo
+ $ cd repo
+ $ for ch in {a..c}; do touch $ch; echo "foo" >> $ch; hg ci -Aqm "Added "$ch;
done
+
+ $ hg log -G -T '{rev}:{node} {desc}' --hidden
+ @ 2:28ad74487de9599d00d81085be739c61fc340652 Added c
+ |
+ o 1:29becc82797a4bc11ec8880b58eaecd2ab3e7760 Added b
+ |
+ o 0:18d04c59bb5d2d4090ad9a5b59bd6274adb63add Added a
+
+ $ echo "bar" >> c
+ $ hg amend
+
+ $ hg log -G -T '{rev}:{node} {desc}' --hidden
+ @ 3:2443a0e664694756d8b435d06b6ad84f941b6fc0 Added c
+ |
+ | x 2:28ad74487de9599d00d81085be739c61fc340652 Added c
+ |/
+ o 1:29becc82797a4bc11ec8880b58eaecd2ab3e7760 Added b
+ |
+ o 0:18d04c59bb5d2d4090ad9a5b59bd6274adb63add Added a
+
+Testing read only commands on the hidden revision
+
+Testing with rev number
+
+ $ hg exp 2
+ abort: hidden revision '2'!
+ (use --hidden to access hidden revisions)
+ [255]
+
+Testing with hash
+
+ $ hg exp 28ad74
+ # HG changeset patch
+ # User test
+ # Date 0 0
+ # Thu Jan 01 00:00:00 1970 +
+ # Node ID 28ad74487de9599d00d81085be739c61fc340652
+ # Parent 29becc82797a4bc11ec8880b58eaecd2ab3e7760
+ Added c
+
+ diff -r 29becc82797a -r 28ad74487de9 c
+ --- /dev/nullThu Jan 01 00:00:00 1970 +
+ +++ b/c Thu Jan 01 00:00:00 1970 +
+ @@ -0,0 +1,1 @@
+ +foo
+
+Make sure things with cmdtype undefined dont work with hidden commits
+
+ $ hg log -r 28ad74
+ abort: hidden revision '28ad74'!
+ (use --hidden to access hidden revisions)
+ [255]
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -61,6 +61,8 @@
release = lockmod.release
+readonly = registrar.command.readonly
+
table = {}
table.update(debugcommandsmod.command._table)
@@ -1863,7 +1865,8 @@
('', 'switch-parent', None, _('diff against the second parent')),
('r', 'rev', [], _('revisions to export'), _('REV')),
] + diffopts,
-_('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'))
+_('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'),
+cmdtype=readonly)
def export(ui, repo, *changesets, **opts):
"""dump the header and diffs for one or more changesets
To: pulkit, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
