Page MenuHomePhabricator

Arcanist fails to load configuration when user's AppData is at a UNC path
Open, Needs TriagePublic

Description

I'll freely admit that my configuration in this matter may be a little bit weird, but this possibly affects any file operation that arcanist performs, and is probably not limited to my configuration.

To reproduce (please do this on a user account on Windows that you don't care about, I'd hate for you to mess up your own account!), open the file properties on the folder C:\Users\<username>\AppData\Roaming, go to the Location tab, and change the location to some network share in the form \\server\share\folder\ etc. You may need to log out and log in again for the change to take effect. Now run arc version --trace to get an output similar to this:

arc version --trace
 ARGV  "c:/apps/arcanist/bin/../scripts/arcanist.php" "version" "--trace"
 LOAD  Loaded "phutil" from "C:\apps\libphutil\src".
 LOAD  Loaded "arcanist" from "C:\apps\arcanist\src".
Config: Reading user configuration file "\\NINETALES\Homes\stwalkerster\AppData\Roaming/.arcrc"...

[2016-04-13 23:44:29] EXCEPTION: (FilesystemException) File system entity 'C:\Users\stwalkerster\Projects\helpmebot\NINETALES\Homes\stwalkerster\AppData\Roaming/.arcrc' does not exist. at [<phutil>\src\filesystem\Filesystem.php:1013]
arcanist(head=stable, ref.master=76ae02325dd3, ref.stable=68f4a77d42c5), phutil(head=stable, ref.master=65b877383e0e, ref.stable=a6d22b5cc662)
  #0 Filesystem::assertExists(string) called at [<phutil>\src\filesystem\Filesystem.php:37]
  #1 Filesystem::readFile(string) called at [<arcanist>\src\configuration\ArcanistConfigurationManager.php:201]
  #2 ArcanistConfigurationManager::readUserConfigurationFile() called at [<arcanist>\src\configuration\ArcanistConfigurationManager.php:263]
  #3 ArcanistConfigurationManager::readUserArcConfig() called at [<arcanist>\scripts\arcanist.php:115]

In the example above, my current working directory is C:\Users\stwalkerster\Projects\helpmebot\ and my roaming AppData folder is located at \\NINETALES\Homes\stwalkerster\AppData\Roaming.

Relevant versions are a6d22b5cc662079dc991be306565871f1d50afb1 for libphutil and 68f4a77d42c5c570176fa7420aa655a4760de2c6 for arcanist.

Having traced the issue on my system, it appears that Filesystem::isAbsolutePath(...) returns false for UNC paths, and as such the current working directory is prepended to the UNC path.

The fix is probably simple - modifying the regex in the Filesystem::isAbsolutePath(...) function to /^([A-Za-z]+:)|(\\\\)/ has appeared to resolve the issue for me, but I'm unaware of what impact that will have elsewhere since this appears to be a fundamental class.

Event Timeline

I suspect that fix is safe and correct. It would be nice to kick the tires a little more before applying it and maybe try to dream up some test coverage, but it shouldn't have broader implications within the arc ecosystem.

(It could have implications if some kind of Windows stuff treats these paths differently in some obscure context, but we can probably cross that bridge when we come to it. It's hard to imagine any undiscovered behavior is disastrously bad or destructive.)

Since Windows tends to languish near the bottom of the priority list and I'm fairly confident this patch is fine, I'll accept it upstream if you use it for a couple days and nothing explodes and want to submit it.