Page MenuHomePhabricator

Remove arcanist projects from working copy code
ClosedPublic

Authored by joshuaspence on May 20 2015, 1:45 AM.
Tags
None
Referenced Files
F13084150: D12945.diff
Wed, Apr 24, 10:49 PM
Unknown Object (File)
Mon, Apr 8, 4:14 PM
Unknown Object (File)
Mon, Apr 8, 4:14 PM
Unknown Object (File)
Mon, Apr 8, 2:32 PM
Unknown Object (File)
Mon, Apr 8, 2:32 PM
Unknown Object (File)
Mon, Apr 8, 5:48 AM
Unknown Object (File)
Wed, Apr 3, 4:55 PM
Unknown Object (File)
Wed, Apr 3, 2:22 PM
Subscribers

Details

Summary

Ref T7604. Remove "arcanist projects" from ArcanistWorkingCopy and a few other callsites. Depends on D12999.

Test Plan

Can't really think of how to test this.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

joshuaspence retitled this revision from to Remove a bunch of "arcanist project" logic from Arcanist.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
epriestley added a reviewer: epriestley.

Looks generally good, but conflated with D12971 in its current state.

I don't have a great fix for the arcanist/libphutil thing. We could put some magic file in each repository, I guess, or try to parse it out of src/__phutil_library_init__.php without actually executing that file, or put some magic comment in that file for arc/libphutil. The use case seems so narrow that I don't think there's much value in trying to build some more-general mechanism.

We could also remove the reentrant code entirely. Maybe confusion about this behavior will be less common than it once was? In particular, two changes may have made it less common:

  • The community is now much less Facebook-heavy, and Facebook deployed arc in a way that made this error (running one copy and thinking you were running a different copy) particularly easy to make.
  • arc ... --trace now shows where things are loading from in the first few lines of output.
This revision now requires changes to proceed.May 25 2015, 12:03 PM

If I understand correctly, the reentrant code means that running /path/to/arc within an arcanist working copy loads the arcanist code from the working copy? If so, I'd favour removing this code.

Although, it wouldn't be too difficult to parse __phutil_library_init__.php and pull the library name from there.

If I understand correctly, the reentrant code means that running /path/to/arc within an arcanist working copy loads the arcanist code from the working copy? If so, I'd favour removing this code.

Yes. Problem was people making changes to arc and then running:

~/workspace/repos/arcanist/ $ arc list

This would invoke /usr/local/arcanist/bin/arc or something and not actually run their new code, which was confusing enough that I wrote the reentrant stuff. But I'm fine with removing it.

joshuaspence retitled this revision from Remove a bunch of "arcanist project" logic from Arcanist to Remove arcanist projects from working copy code.May 25 2015, 12:22 PM
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence edited edge metadata.
joshuaspence removed a subscriber: Korvin.
epriestley edited edge metadata.
This revision is now accepted and ready to land.May 25 2015, 2:31 PM
This revision was automatically updated to reflect the committed changes.