Page MenuHomePhabricator

Various fixes for hosted and non-hosted subversion queries in Diffusion.
ClosedPublic

Authored by wotte on Dec 19 2013, 3:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 14, 11:18 PM
Unknown Object (File)
Sat, Dec 14, 12:55 PM
Unknown Object (File)
Thu, Nov 28, 9:58 PM
Unknown Object (File)
Mon, Nov 25, 2:38 AM
Unknown Object (File)
Sun, Nov 24, 1:21 PM
Unknown Object (File)
Thu, Nov 21, 12:22 AM
Unknown Object (File)
Nov 16 2024, 6:18 PM
Unknown Object (File)
Nov 12 2024, 1:29 PM

Details

Summary

There were a number of places that were generating nonsense queries for both hosted and non-hosted subversion repositories.

Test Plan

Attempted several activities in Diffusion with both a hosted and non-hosted subversion repository, including viewing various types of diffs and raw files.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Note that changes to DiffusionSvnRawDiffQuery.php are speculative, as I couldn't find anything that actually called this.

See inline -- getSubversionPathURI() is intended to simplify this construction further, provided I implemented it correctly. These changes are correct conceptually, but I think they can be mechanically simpler.

(The root issue here is a base URI without a trailing /, which we used to require explicitly but no longer do under the theory that everything is using getSubversionPathURI(), although obviously that's not entirely true.)

src/applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php
196

You should be able to use a single %s, with:

$repository->getSubversionPathURI($ref, $rev)

...in theory.

wotte updated this revision to Unknown Object (????).Dec 19 2013, 4:11 PM

Updated based on @epriestley feedback. Some light testing as above.

Changes in DiffusionSvnRawDiffQuery.php remain speculative, as I don't know how to test it.

The easiest way to test the RawDiff stuff is by calling the method through the /conduit/ console. It works correctly for me with this patch applied:

{F90864}

These objects are created somewhat-dynamically by DiffusionRawDiffQuery, which is why it's difficult to identify how they're called. We're moving away from this older style of organization (since it was generally bad for a lot of reasons) but this particular piece of code still has the vestigial, hard-to-grep-for format.

I've added you (and your other account) to Blessed Committers, which gives you commit access:

  1. In Settings > SSH Public Keys here in the web interface, add an SSH public key.
  2. Align your repository's origin to the authoritative one on this server, if you're currently pulling from GitHub: git remote set-url origin ssh://dweller@secure.phabricator.com/diffusion/P/
  3. Contemplate great power / great responsibility / etc.
  4. git push

I can also just pull this if that's a pain to setup for the moment.

Thanks for fixing this!

(Well, (4) should maybe be arc land or arc amend + git push.)

Thanks! (Not entirely certain how @wrotte got created)