hmm, CPPFLAGS here is passed to C++ compilers instead of preprocessors. Should we rename it to CXXFLAGS entirely?

@valerio.bozzolan Can this be closed nowadays?

AOSC OS you said? Learned a new thing. So, approved.

IIRC, CPPFLAGS are passed to the preprocessor while CXXFLAGS are passed to the C++ compiler...
In any case, += syntax should add nicely to previously empty flags.

Does it still work if there are not compilation flags in CXXFLAGS? I assume yes, but I don't know anything about makefiles lol

We end up in the depths of ArcanistBundle::newBase85Data() here.
Which tries to copy behavior of https://github.com/git/git/blob/master/diff.c#L3325-L3395 which may have changed over time (or not).
I'm still wondering if this is a change in git (the software) creating a binary diff which Phorge then compares to https://we.phorge.it/source/arcanist/browse/master/src/parser/__tests__/patches//5dec8bf28557f078d1987c4e8cfb53d08310f522.gitpatch.

@valerio.bozzolan See last code block in the task description

OK, tested by 3 hackers, no git regressions, more SVN stability, more inline documentation, more abstraction, very Phorgi, already tracking follow-ups, yuuup

In T15957#22436, @keithzg wrote:

Testing this a bit myself again, I discovered that actually this problem affects both Git and SVN

Testing this a bit myself again, I discovered that actually this problem affects both Git and SVN, it just wasn't being noticed in my SVN testing because it doesn't go wrong if run from any subfolder. So for example in a checkout of rARC, running arc browse . in the root directory will pop one into the bogus path of, say, https://we.phorge.it/source/arcanist/browse/master/home/keithzg/Code/git/arcanist, but if I'm in my local ~/Code/git/arcanist/src/ then arc browse . pops open https://we.phorge.it/source/arcanist/browse/master/src as intended.

The only remaining issue I could find in retesting was that, when run on the root of a repo, apparently SVN exhibits the same behaviour as Git does in T15957; and furthermore in my testing actually Git also doesn't have that problem if arc browse . is run in a subfolder rather than the root of a repo. (It's just so much more common to be in the root of a Git repo, and so much less common to be in the true root of an SVN repo, that we didn't notice!)

Closed all inline comments.

• adopt suggested indentation in ArcanistRepositoryRef.php:91-94
• adopt explicit "TODO" in ArcanistRepositoryRef.php:138-139

Reading (and testing) this again/more, after our first round in person a few days ago, now that I have a local SVN repo, I cannot find anything wrong or non-working, still I'd ideally appreciate another pair of eyes.
Added two more small comments.

ArcanistRepositoryAPI: apply suggested inline comment:

look mum, I'm learning English

improve comments (again) - thanks

avoid double unset(), just use or

update inline doc, thanks aklapper

Seems to work locally.

@mturdus I have landed this but have reintroduced this code which you have removed - please accept my fix: D25960

@valerio.bozzolan could you please add to either H28 or H29 Affected files contains none of map.php?

The code to select which tests to run by default is in ArcanistWorkflow::selectPathsForWorkflow().