Summary: `PhutilFileLockException` does not exist. Per https://we.phorge.it/source/arcanist/browse/master/src/filesystem/PhutilFileLock.php$54-66 , if lock acquisition fails it is supposed to throw a `PhutilLockException` instead.
Test Plan:
Grep the code base.
Or, run this, from arcanist, from two different terminals:
php ./support/unit/lock.php asd.txt
After this change, the exception is correctly managed.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25641
Summary:
Looks like this is all that's needed?
Ref T15751
Test Plan:
R12 has some scenarios for testing this.
Also ran `arc liberate --clean` on arc and phorge repos, and the generated map did not change.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15751
Differential Revision: https://we.phorge.it/D25551
Summary:
Mercurial 6.4 was recently released and showing up in package managers. With
the update to 6.4 using `arc land` would result in an exception indicating that
`expandpath` function does not exist.
The `ui.expandpath` function was deprecated in 5.8 and now removed in 6.4. The
functionality has been moved to `utils.urlutil.get_` functions (they are split
between getting pull, push, and clone paths).
This updates the script to try `utils.urlutil.get_clone_path` function if the
`ui.expandpath` function is not present.
Imported from:
https://secure.phabricator.com/rARC0fc22183e796fb8ac2e3a0a3f3f37aa964c6d7fa
Test Plan:
I updated my latest mercurial install to 6.4 and verified with `hg --version`.
I created a diff in a mercurial repo and used `arc land` to successfully land
the revision without any exceptions.
Closes T15288
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15288
Differential Revision: https://we.phorge.it/D25143
Summary:
Updated the mercurial script to pull the `parseurl` function from the new module if pulling from the old module fails.
Also updated pulling of `remoteopts` to follow the same pattern of fallback.
Fixes T13672
Test Plan:
Using mercurial 6.1 I ran `arc patch Dnnnnn --trace` and verified that it succeeded and in the trace output it used the `arc-ls-markers` extension.
Using mercurial 4.7 I ran `arc patch Dnnnnn --trace` and verified that it succeeded and in the trace output it used the `arc-ls-markers` extension.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T13672
Differential Revision: https://secure.phabricator.com/D21747
Summary: Ref T13588. I pointed my local `php` at PHP 8.1 and this is what I've hit so far; all these cases seem very unlikely to have any subtle behavior.
Test Plan: Ran various `arc` workflows under PHP 8.1.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21742
Summary:
Reported at the phorge project (https://we.phorge.it/D25017), running `arc liberate` fails on PHP 8 due to the `log()` function using `fwrite()` incorrectly assuming a format pattern can be used.
This updates to remove most of these status messages are they are largely uninformative and instead we can report progress.
- Remove the `--quiet` argument
- Always display the progress
- Remove all informational/status log statements
Test Plan:
Tested using both PHP 7.3 and PHP 8:
1. I ran `arc liberate` and saw the standard output:
```lang=console
SCAN Searching for libraries in the current working directory...
WORK Updating library: src/
Done.
DONE Updated library.
```
2. I ran deleted `phabricator/src/.phutil_module_cache` and ran `arc liberate /src`, verifying that progress was displayed while the map was computed.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21718
Summary:
After `arc diff` creates a revision in Phabricator it amends the commit to include a link to the revision in the commit message. For Mercurial this is done with `hg commit --amend --logfile` however this will fail when trying to create a diff for a non-head commit.
This updates `ArcanistMercurialAPI::amendCommit()` to allow amending a non-head commit in two ways, depending on whether `evolve` is in use:
No evolve:
1. Rebasing the current commit onto the current commit's parent, using the new commit message
2. Rebasing all children + descendants of the current commit onto the new resulting commit
3. Stripping the original commit
With evolve:
1. Amend the commit with `hg amend --logfile`
2. Run `hg evolve` to tidy up all commits
Test Plan:
I created 6 commits in a row placing a bookmark at commits 2 `bookmark1`, 4 `bookmark2`, and 6 `bookmark3`, and ensured I had `arc:bookmark` in my base ruleset.
No evolve, non-head changeset:
1. I verified I did not have `evolve` enabled by running `hg debugextensions` and did not see `evolve` in the listed active extensions.
2. I updated to `bookmark1` and modified a file to leave a dirty working state.
3. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
4. I checked the status of my repository and verified it was still linear and the bookmarks pointed to the proper commits.
5. I ran `hg log -r bookmark1 --template {desc}` to view the full commit message and verified it contained both `Summary: ...` and `Differential Revision: https://...`.
6. I ran `hg diff -c bookmark1` and verified the changes for that commit included the changes I made in step 2.
No evolve, head changeset:
1. I updated to `bookmark3` which is the head commit and modified a file to leave a dirty working state.
2. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
3. I checked the status of my repository and verified it was still linear and all the bookmarks pointed to the proper commits.
4. I ran `hg log -r bookmark3 --template {desc}` to view the full commit message and verified it contained both `Summary: ...` and `Differential Revision: https://...`.
5. I ran `hg diff -c bookmar3` and verified the changes for that commit included the changes I made in step 1.
With evolve:
1. I enabled `evolve` and verified it was enabled by running `hg debugextensions` and saw `evolve` in the listed active extensions.
2. I updated to `bookmark2` and modified a file to leave a dirty working state.
3. I ran `arc diff` and when prompted to amend my changes I said "yes", and verified a phab revision was created properly.
4. I checked the status of my repository and verified it was still linear and all the bookmarks pointed to the proper commits.
5. I ran `hg log -r bookmark2 --template {desc}` to view the full commit message and verfieid it contained both `Summary: ...` and `Differential Revision: https://...`.
6. I ran `hg diff -c bookmark2` and verified the changes for that commit included the changes I made in step 2.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21686
Summary: See PHI2085. Python 3 is stricter about integers and floats than Python 2 was, and we can end up passing a float where an integer was expected if the player wins "arc anoid" using a terminal with certain (most?) character dimensions.
Test Plan:
- Modified "arcanoid.py" to win instantly.
- Adjusted terminal window to 80x24, ran "arc anoid", reproduced crash.
- Ran "python2 arcanoid.py" and observed old victory animation behavior.
- Applied patch.
- Ran "arc anoid" and observed identical victory animation behavior.
Differential Revision: https://secure.phabricator.com/D21667
Summary:
Ref T13100. Ref T13586. These options have been in production for a while and seem to consistently improve behavior, downgrading segfaults to runtime errors.
Make them global options for all CLI environments. See next change for web enviornments.
Test Plan: Ran `arc`.
Maniphest Tasks: T13586, T13100
Differential Revision: https://secure.phabricator.com/D21566
Summary: Ref T13598. This builds on D21537 and adds support for correcting the capitalization of builtin systems.
Test Plan:
- Linted a file that uses "ExCePtIoN", got a lint correction.
Maniphest Tasks: T13598
Differential Revision: https://secure.phabricator.com/D21538
Summary:
As of PHP 8, the XML entity loader is disabled by default and the
libxml_disable_entity_loader function is deprecated. Thus suppress the
deprecation warning for now; we could skip the function call, but this
is safer.
Test Plan: Used to create this revision with PHP 8 on macOS
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D21497
Summary:
See PHI1894. PHP 7.4 introduced a new runtime configuration option, "zend.exception_ignore_args", which removes the "args" from exception backtraces.
The "PhutilOpaqueEnvelopeTestCase" relies on this behavior (since it explicitly inspects stack frames). Although the test isn't critical and could be restructured, it seems like there is little value to ever enabling this option in the context of Phabricator.
Disable it at startup so environments are more consistent across different PHP versions and configurations.
Test Plan:
- Enabled "zend.exception_ignore_args" under PHP 7.4.
- Ran "PhutilOpaqueEnvelopeTestCase".
- Before: failure, expected signpost value not present in stack trace (because no "args" are present on the exception).
- After: test passes.
Differential Revision: https://secure.phabricator.com/D21473
Summary:
Ref PHI1808. In Mercurial, we must save and restore bookmark state explicitly.
- Save and restore bookmarks.
- Clean up concepts in "arc-ls-markers" slightly, so we don't need separate "isCurrent" and "isActive" flags, hopefully.
I believe the totality of Mercurial state is:
- A (non-bare) working copy points at exactly one commit (which might be the empty/null commit, in an empty repository).
- A working copy has exactly one active branch.
- Each branch has zero or more heads.
- Each head may be closed.
- Each (non-null) commit belongs to exactly one branch.
- Note that the active branch may have zero heads and zero commits which belong to it!
- A working copy has zero or one active bookmark.
To capture this, we now emit:
- A list of branch heads. If a branch head is a working copy commit, that head is flagged as active.
- A list of bookmarks. If a bookmark is the current bookmark, that bookmark is flagged as active.
- A single "branch-state" virtual marker. This covers the case where you have run "hg branch X" to create X, but no objects in the working copy actually correspond to X yet. It also covers the case where you are on a concrete branch, but not any head of that branch.
- A single "commit-state" virtual marker. This always shows the current commit in the working copy.
Test Plan:
- Useful states to test are:
- Empty repository (not all commands currently work here).
- Normal repository, on a bookmark.
- Normal repository, no bookmark.
- "hg up 123" to update to somewhere in history.
- "hg branch X", to start a new branch with no commits.
- Ran "arc branches" and "arc bookmarks" in various states. Saw generally sensible output.
- Ran "arc land --hold ..." in various states against a failing remote. Saw generally sensible output, and saw working properly restored to the original state.
Differential Revision: https://secure.phabricator.com/D21396
Summary:
Ref PHI1805. Under some combination of versions (Python 3.8?), "arc-ls-markers" is running into additional Python runtime issues.
Sprinkle more "b" around to resolve them? Also clean up a couple of plain "arc" issues.
Test Plan:
Landed a change in Mercurial.
Some of this works fine without changes in Python 3.7/2.7 against Mercurial 4.7/5.4, so this may not be exhaustive.
Differential Revision: https://secure.phabricator.com/D21393
Summary:
Ref T13546. See PHI1805. Currently, the "arc-ls-markers" extension doesn't run under Python 3:
- some stuff needs "b'...'" to mark it as a byte string;
- "dict.iteritems()" is gone in Python 3, and "mercurial.pycompat" isn't always available;
- in Python 3, "json" refuses to print byte strings; and
- the compiler caching behavior in Python 3 has changed.
Try to get these things working in the same way under Python 2 and Python 3.
Test Plan:
Ran this command (with `python` as Python 2, locally):
```
$ python /usr/local/bin/hg --config 'extensions.arc-hg=/Users/epriestley/dev/core/lib/arcanist/support/hg/arc-hg.py' arc-ls-markers --
```
...and this command:
```
$ python3 /usr/local/bin/hg --config 'extensions.arc-hg=/Users/epriestley/dev/core/lib/arcanist/support/hg/arc-hg.py' arc-ls-markers --
```
..and saw the same output in both cases (previously, `python3 ...` fataled in various ways).
Maniphest Tasks: T13546
Differential Revision: https://secure.phabricator.com/D21392
Summary:
Ref T9948. Ref T13546. To identify remote branch heads -- not just modified heads -- use "branchmap" like "hg outgoing" does.
I wasn't able to find any other way to get what we want: for example, with a bundlerepo, "peer.heads()" and "peer.changelog.heads()" include local branches which are not present in the remote.
It generally seems difficult (perhaps impossible?) to distinguish between these cases by using "getremotechanges()":
- branch X exists at position Y in both the local and remote;
- branch X exists at positino Y in the local, but not the remote.
In any case, this seems to work properly and //should// do less work than "getremotechanges()" since we don't need to pull as much data over the wire.
Test Plan: Ran "hg arc-ls-markers" in various repositories, got what appeared to be a faithful representation of the remote branch and bookmark state.
Maniphest Tasks: T13546, T9948
Differential Revision: https://secure.phabricator.com/D21349
Summary:
Ref T9948. Ref T13546. This change moves toward a functional "arc land" in Mercurial.
Because of how "bundlerepo.getremotechanges()" works, "hg arc-ls-markers" does not actually list markers in the remote that aren't different from local markers so it's hard to get anywhere with this.
Test Plan: Got somewhat-encouraging output from "arc land" and "hg arc-ls-markers", but too many things are still broken for this to really work yet.
Maniphest Tasks: T13546, T9948
Differential Revision: https://secure.phabricator.com/D21348
Summary: Ref T13546. Ref T9948. It seems challenging to examine a remote in vanilla Mercurial. Provide an "hg arc-ls-remote" command which functions like "git ls-remote" so we can figure out if "--into X" is a bookmark, branch, both, neither, or a branch with multiple heads without mutating the working copy as a side effect.
Test Plan: Ran various "arc land --into ..." commands in a Mercurial working copy, saw apparently-sensible resolution of remote marker names.
Maniphest Tasks: T13546, T9948
Differential Revision: https://secure.phabricator.com/D21343
Summary:
Ref T13490. Bring "arc prompts" from "wilds" and hook it into the prompt in "arc shell-complete".
See D21069. Fix an issue where the shell hook tested for a path other than the path it writes to.
Test Plan: Ran "arc shell-complete" with no hook and got a prompt. Shell completed things. Ran "arc prompts shell-complete".
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21085
Summary:
Depends on D21067. Ref T13492. Converting unit tests to be readable exposed this error in the grammar.
Normally, "grammar_rule" rules emit a standalone Node. In this case, the bottom-level grammar rule is a collection of trivial rules and the callers configure the Node. Wrap the bottom-level rule in a configuration rule so the node is configured correctly and consistently, in exactly one place.
Test Plan: Ran unit tests.
Maniphest Tasks: T13492
Differential Revision: https://secure.phabricator.com/D21068
Summary:
Depends on D21066. Ref T13492. The switch of unit test data to stable/readable output exposed this bug in parsing of variadic calls: some nodes are not given types properly.
Fix the parser and update the test.
Test Plan: Ran the test, which now works.
Maniphest Tasks: T13492
Differential Revision: https://secure.phabricator.com/D21067
Summary:
Depends on D21063. Ref T13492. Currently, XHPAST defines a version in both PHP code and C code, and they must be kept in sync.
Switch to a single definition in PHP, then carry it through the build pipeline into C.
Test Plan: Did a clean rebuild of XHPAST, saw a version number carried in from PHP. Ran "xhpast --version".
Maniphest Tasks: T13492
Differential Revision: https://secure.phabricator.com/D21064
Summary:
Fixes T9753. Changes some time ago (in D13970 + D13974) improved XHPAST build compile-time warning behavior under Bison 3.
However, macOS still ships with Bison 2.3 and these changes prevent XHPAST from building with Bison 2.3. The changes didn't introduce version detection, so Bison 2.3 builds fail somewhat mysteriously without obvious next steps.
It's relatively easy to install Bison 3 on macOS via Homebrew, but the Bison 3 changes aren't terribly substantive and XHPAST doesn't actually depend on any Bison 3 features, so just return to Bison 2.3 for now.
It would be reasonable to undo this again and retarget Bison 3 in the future, but ideally we should wait until macOS ships with Bison 3 or we have a specific reason to bump the minimum required version to 3. If/when we do, we should version-detect Bison and raise a clear error message.
Test Plan: Built xhpast under Bison 2.3 on a default macOS install using "make cleanall && make install".
Maniphest Tasks: T9753
Differential Revision: https://secure.phabricator.com/D21063
Summary: Ref T13490. Update the "weld" workflow and the "anoid" workflow. Incorporates D20938.
Test Plan: Ran "arc weld". Ran "arc anoid".
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21001
Summary:
Depends on D20988. Ref T13395. Ref T13098. Ref T13203.
This brings all the "toolsets" code into "master". We try to run commands as toolsets commands first, then fall back to older code.
Since the "toolsets" class tree is mostly parallel to the older class tree, this isn't completely broken. Currently, all commands fall back.
Test Plan: Created this diff, ran various other commands. But this is probably a long shot from finished.
Maniphest Tasks: T13395, T13203, T13098
Differential Revision: https://secure.phabricator.com/D20990
Summary:
Ref T13395. Merge a lot of stuff which doesn't break existing workflows:
- Merge a UTF8 fix for "cmd.exe" on Windows.
- Merge minor changes to JSON linters.
- Merge some shell completion stuff.
- Merge some "arc anoid" fixes.
- Merge various Windows improvements to unit tests which interact with processes / the filesystem.
- Merge some other Windows path fixes.
- Merge a UTF8 character class fix.
- Merge script initialization.
- Merge unit test support scripts.
- Merge some initialization code.
- Merge Windows stdout/stderr-as-files code.
- Merge a bunch of code for making exec tests work on Windows.
- Merge more Windows unit test fixes.
- Merge "continue on failure" mode when loading symbols.
- Merge "GPC" order CLI fixes.
Test Plan: Ran `arc unit --everything`; created this change. There's likely some less-than-perfect code here.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20988
Summary: Ref T13395. Fixes a file-locking test and removes a logging test that's more trouble than it's worth.
Test Plan: Ran "arc unit --everything" locally, got a clean bill of health.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20984
Summary: Ref T13395. Moves all remaining code in "libphutil/" into "arcanist/".
Test Plan: Ran various arc workflows, although this probably has some remaining rough edges.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20980
