Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7604: Remove "Arcanist Projects"
- Commits
- rARC64d03ff68bf2: Remove arcanist projects from working copy code
Can't really think of how to test this.
Diff Detail
- Repository
- rARC Arcanist
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 6258 Build 6280: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
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.
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 listThis 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.