Skip to content
Commits on Apr 30, 2016
  1. @epriestley

    Fix "acccess" spelling error

    epriestley committed Apr 30, 2016
    Summary: This is misspelled.
    
    Test Plan: Consulted a dictionary.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Differential Revision: https://secure.phabricator.com/D15827
  2. @epriestley

    Flesh out more web UI actions for new URI interface

    epriestley committed Apr 29, 2016
    Summary:
    Ref T10748.
    
      - Allow users to add new URIs by clicking a button instead of knowing a secret URI.
      - Validate that URIs are actually valid URIs.
      - Add enable/disable action and strings.
    
    Test Plan:
      - Created a new URI.
      - Tried to create a nonsense URI, created a good URI.
      - Enabled/disabled a URI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15825
Commits on Apr 29, 2016
  1. @epriestley

    Alternate fix for Firefox triple click selection

    epriestley committed Apr 29, 2016
    Summary:
    Fixes T10905. This reverts D15823, which didn't work well for tasks with very long titles (the title would break as a block element).
    
    This is slightly more magic but works with long titles.
    
    Test Plan: Did everything from D15823, but also with long titles. Triple-click, wrapping, and mobile/device worked in Safari, Firefox and Chrome.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10905
    
    Differential Revision: https://secure.phabricator.com/D15824
  2. @epriestley

    Allow tall two-column/curtain headers to be triple-clicked in Firefox

    epriestley committed Apr 29, 2016
    Summary:
    Fixes T10905. In Firefox, triple clicking the new headers doesn't select the entire line, so you can't easily copy/paste an entire task title or revision name. It works fine in Safari/Chrome.
    
    This seems to fix that without breaking anything.
    
    Test Plan:
      - Viewed headers in Safari, Firefox, Chrome.
      - Triple-clicked headers in Safari, Firefox, Chrome.
      - Viewed tablet/device layouts in Safari, Firefox, Chrome.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10905
    
    Differential Revision: https://secure.phabricator.com/D15823
  3. @epriestley

    Allow callers to query information about repository URIs from diffusi…

    epriestley committed Apr 29, 2016
    …on.repository.search
    
    Summary: Ref T10748. Adds a "uris" attachment with URI information.
    
    Test Plan: Queried URI information via Conduit, saw reasonable looking information.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15822
  4. @epriestley

    Add `diffusion.uri.edit` for creating and editing repository URIs

    epriestley committed Apr 29, 2016
    Summary: Ref T10748. Brings the rest of the transactions to EditEngine, supports creating via API.
    
    Test Plan:
      - Created a URI via API.
      - Created a URI via web.
      - Tried to apply sneaky transactions, got rejected with good error messages. <_< >_>
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15821
  5. @epriestley

    Document all the hypothetical URI features we plan to support soon

    epriestley committed Apr 28, 2016
    Summary: Ref T10748. Ref T10366. This documents how everything is planned to work shortly.
    
    Test Plan: Read documentation.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Subscribers: eadler, scode
    
    Maniphest Tasks: T10366, T10748
    
    Differential Revision: https://secure.phabricator.com/D15817
  6. @epriestley

    Add repository URI view pages and IO/Display edit logic

    epriestley committed Apr 28, 2016
    Summary:
    Ref T10748.
    
      - New View page for repository URIs.
      - Make display and I/O behavior (observe, mirror, read, read/write) editable.
      - Add a bunch of checks to prevent you from completely screwing up a repository by making it writable from a bunch of differnet sources.
    
    Test Plan:
    {F1249866}
    
    {F1249867}
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15816
  7. @epriestley

    Rough sketch of new repository URI editing

    epriestley committed Apr 28, 2016
    Summary:
    Ref T10748. Ref T10366. This adds a new EditEngine, EditController, Editor, Query, and Transaction for RepositoryURIs.
    
    None of these really do anything helpful yet, and these URIs are still unused in the actual application.
    
    Test Plan: {F1249794}
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10366, T10748
    
    Differential Revision: https://secure.phabricator.com/D15815
  8. @epriestley

    Give users a modal VCS choice when creating a new repository

    epriestley committed Apr 27, 2016
    Summary:
    Ref T10748. Allow the new EditEngine workflow to create repositories by giving the user a modal repository type choice upfront.
    
    (The rest of this flow is still confusing/weird, though.)
    
    Test Plan:
      - Created a new repository.
    
    {F1249626}
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15813
Commits on Apr 28, 2016
  1. @epriestley

    Port "Actions" to new Repository UI

    epriestley committed Apr 27, 2016
    Summary: Ref T10748. This brings the "Actions" items (publish/notify + autoclose enabled) into the new UI.
    
    Test Plan:
      - Edited this stuff via EditEngine and Conduit.
      - Viewed via new Manage UI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15811
  2. @epriestley

    Port Repository "Branches" to new UI

    epriestley committed Apr 27, 2016
    Summary: Ref T10748. Makes a "Branches" panel, enables these transactions in the EditEngine.
    
    Test Plan:
      - Edited via EditEngine + Conduit.
      - Viewed via manage UI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15809
  3. @epriestley

    Port Repository "Symbols" to Manage/Panel UI

    epriestley committed Apr 27, 2016
    Summary: Ref T10748. Port this, add EditEngine support, add some type validation to the transaction.
    
    Test Plan:
      - Edited via EditEngine.
      - Edited via Conduit.
      - Viewed via Management UI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15808
Commits on Apr 27, 2016
  1. @epriestley

    Improve cache behaviors for font files and other nonstandard resource…

    epriestley committed Apr 27, 2016
    … types
    
    Summary:
    Ref T10843. There are actually two separate notions of cacheability here:
    
      - Is this cacheable by the browser (e.g., should we emit "Expires: long in the future")?
      - Is this cacheable locally (e.g., should we stick it in APC, or just read it off disk every time)?
    
    These got a little mixed up by D15775, so we aren't currently emitting proper "Expires" headers on font files and a few other resource types.
    
    Straighten this out so that we "Expires" these unusual resources correctly.
    
    Test Plan: Verified that `.woff` files get a proper "Expires" header now, not just CSS/JS.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10843
    
    Differential Revision: https://secure.phabricator.com/D15807
  2. @epriestley

    Port "Allow Dangerous Changes" to new Manage UI

    epriestley committed Apr 26, 2016
    Summary: Ref T10748. Brings this forward in the UI and EditEngine.
    
    Test Plan:
      - Edited via Conduit.
      - Viewed via Manage UI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15805
  3. @epriestley

    Port "Automation" panel to new Repository Manage UI

    epriestley committed Apr 26, 2016
    Summary: Ref T10748. Ports this UI and exposes it on the EditEngine.
    
    Test Plan:
      - Edited via EditEngine.
      - Viewed new manage UI.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Subscribers: hach-que
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15804
  4. @epriestley

    Probably improve behavior around duplicate notifications

    epriestley committed Apr 26, 2016
    Summary:
    We're sometimes getting duplicate notifications right now. I think this is because multiple windows are racing and becoming leaders.
    
    Clean this up a little:
    
      - Fix the `timeoout` typo.
      - Only try to usurp once.
      - Use different usurp and expire delays, so we don't fire them at the exact same time.
    
    Not sure if this'll work or not but it should theoretically be a little cleaner.
    
    Test Plan:
      - Quit Safari, reopened Safari, still saw a fast reconnect to the notification server (this is the goal of usurping).
      - Did normal notification stuff like opening a chat in two windows, got notifications.
      - Hard to reproduce the race for sure, but this at least fixes the outright `timeoout` bug.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Differential Revision: https://secure.phabricator.com/D15806
Commits on Apr 26, 2016
  1. @epriestley

    Add an edge table to the `search` database

    epriestley committed Apr 26, 2016
    Summary:
    Fixes T10778. This is a result of T10262: when we save a form configuration and adjust the policy, we try to scramble attached file secrets.
    
    There aren't going to be any attached files, but there's also no edge table, so we fail.
    
    We could skip this code, but we'll likely need an edge table here sooner or later so it's probably simpler in the long run to just add an empty one.
    
    Test Plan:
       - Ran `bin/storage upgrade`, got a clean bill of health.
       - Saved a form configuration after making a policy edit, no more `edge` exception.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10778
    
    Differential Revision: https://secure.phabricator.com/D15803
  2. @epriestley

    Add `bin/repository clusterize` and document setup and migration for …

    epriestley committed Apr 25, 2016
    …clusters
    
    Summary: Ref T4292. This provides at least some sort of hint about how to set up cluster repositories.
    
    Test Plan:
      - Read documentation.
      - Ran `bin/repository clusterize` to add + remove clusters.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T4292
    
    Differential Revision: https://secure.phabricator.com/D15798
  3. @epriestley

    Improve Diffusion hosting setup instructions somewhat?

    epriestley committed Apr 25, 2016
    Summary:
    Ref T10866. Fixes T10386. This attempts to make it a little more plausible to follow these directions:
    
      - Use simpler language in general.
      - Remove language suggesting that HTTP requires no additional configuration.
      - Suggest using a load balancer or an ugly port number instead of swapping SSH to a different port.
      - Be more granular about `sudo` setup.
      - Organize better?
    
    Test Plan: Read documentation.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10386, T10866
    
    Differential Revision: https://secure.phabricator.com/D15796
  4. @epriestley

    When multiple web hosts are in service, don't require setup warnings …

    epriestley committed Apr 26, 2016
    …to be dismissed on each one
    
    Summary:
    Fixes T10876. Currently, we can end up with a setup warning banner sticking on each web device, since the state is stored in local cache.
    
    Instead:
    
      - When we actually run the setup checks, save the current state in the database.
      - Before we show a cached banner, make sure the database still says the checks are a problem.
    
    This could lead to some inconsistencies if setup checks legitimately pass on some hosts but not on others. For example, if you have `git` installed on one machine but not on another, we may raise a setup warning ("No Git Binary!") about it on one host only.
    
    For now, assume users have their operational environments in some sort of reasonable shape and can install the same stuff everywhere. In the future, we could split the issues into "global" and "per-host" issues if we run into problems with this.
    
    Test Plan:
    This is somewhat tricky to test locally since you really need multiple webservers to test it properly, but I:
    
      - Created some setup issues, saw banner.
      - Ignored/cleared them, saw banner go away.
      - Verified database cache writes were occurring properly.
    
    Then I sort of faked it like this:
    
      - Created a setup issue.
      - Manually set the database cache value to `[]` ("no issues").
      - Reloaded page.
      - No more banner.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10876
    
    Differential Revision: https://secure.phabricator.com/D15802
  5. @epriestley

    Port "Staging Area" repository section to new management UI

    epriestley committed Apr 26, 2016
    Summary: Ref T10748. Brings this over and adds EditEngine support for it.
    
    Test Plan: Viewed and edited staging area information.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15801
  6. @epriestley

    Port local storage path to new repository Manage UI

    epriestley committed Apr 26, 2016
    Summary: Ref T10748. This merges "Storage" and "Cluster" into a single UI which combines the information of both.
    
    Test Plan: {F1246882}
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Subscribers: hach-que
    
    Maniphest Tasks: T10748
    
    Differential Revision: https://secure.phabricator.com/D15800
  7. @epriestley

    Prevent web queries from running for more than 30 seconds

    epriestley committed Apr 26, 2016
    Summary:
    Ref T10849. This enforces a global 30-second per-query time limit for anything not coming from the CLI.
    
    If we run into another issue with MySQL hanging in the future, this should prevent it from being nearly as bad as it was.
    
    Test Plan:
      - Set value to 0, verified the UI threw an exception immediately.
      - Set value back to 30, browsed around a bunch of pages.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10849
    
    Differential Revision: https://secure.phabricator.com/D15799
Commits on Apr 25, 2016
  1. @epriestley

    Document how to register cluster devices with Almanac

    epriestley committed Apr 22, 2016
    Summary:
    Ref T4292. This is a required step in configuring a cluster: document and explain it.
    
    Previously `bin/almanac register` could //also// add and trust keys. I've removed this capability since I think it's needless and complicated. If there's some real use for it eventually, we could add a `bin/almanac add-key` or whatever. The workflow is simpler and has better guard rails that point you in the correct direction now.
    
    Test Plan:
      - Read documentation.
      - Ran `bin/almanac` with various good/bad flags.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T4292
    
    Differential Revision: https://secure.phabricator.com/D15795
  2. @epriestley

    Fix two minor formatting issues with `bin/repository move-paths`

    epriestley committed Apr 25, 2016
    Summary: This gets over-escaped instead of bolded right now, but I only ever hit it when exporting/importing and never both cleaning it up.
    
    Test Plan: Ran `bin/repository move-paths`, saw bolded "Move" instead of ANSI escape sequences.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Differential Revision: https://secure.phabricator.com/D15797
  3. @epriestley

    Make cluster repositories more resistant to freezing

    epriestley committed Apr 24, 2016
    Summary:
    Ref T10860. This allows us to recover if the connection to the database is lost during a push.
    
    If we lose the connection to the master database during a push, we would previously freeze the repository. This is very safe, but not very operator-friendly since you have to go manually unfreeze it.
    
    We don't need to be quite this aggressive about freezing things. The repository state is still consistent after we've "upgraded" the lock by setting `isWriting = 1`, so we're actually fine even if we lost the global lock.
    
    Instead of just freezing the repository immediately, sit there in a loop waiting for the master to come back up for a few minutes. If it recovers, we can release the lock and everything will be OK again.
    
    Basically, the changes are:
    
      - If we can't release the lock at first, sit in a loop trying really hard to release it for a while.
      - Add a unique lock identifier so we can be certain we're only releasing //our// lock no matter what else is going on.
      - Do the version reads on the same connection holding the lock, so we can be sure we haven't lost the lock before we do that read.
    
    Test Plan:
      - Added a `sleep(10)` after accepting the write but before releasing the lock so I could run `mysqld stop` and force this issue to occur.
      - Pushed like this:
    
    ```
    $ echo D >> record && git commit -am D && git push
    [master 707ecc3] D
     1 file changed, 1 insertion(+)
    # Push received by "local001.phacility.net", forwarding to cluster host.
    # Waiting up to 120 second(s) for a cluster write lock...
    # Acquired write lock immediately.
    # Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
    # Acquired read lock immediately.
    # Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
    # Ready to receive on cluster host "local001.phacility.net".
    Counting objects: 3, done.
    Delta compression using up to 8 threads.
    Compressing objects: 100% (2/2), done.
    Writing objects: 100% (3/3), 254 bytes | 0 bytes/s, done.
    Total 3 (delta 1), reused 0 (delta 0)
    BEGIN SLEEP
    ```
    
      - Here, I stopped `mysqld` from the CLI in another terminal window.
    
    ```
    END SLEEP
    # CRITICAL. Failed to release cluster write lock!
    # The connection to the master database was lost while receiving the write.
    # This process will spend 300 more second(s) attempting to recover, then give up.
    ```
    
      - Here, I started `mysqld` again.
    
    ```
    # RECOVERED. Link to master database was restored.
    # Released cluster write lock.
    To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
       2cbf87c..707ecc3  master -> master
    ```
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10860
    
    Differential Revision: https://secure.phabricator.com/D15792
  4. @epriestley

    Make cluster repositories more chatty

    epriestley committed Apr 24, 2016
    Summary:
    Ref T10860. At least in Git over SSH, we can freely echo a bunch of stuff to stderr and Git will print it to the console, so we can tell users what's going on.
    
    This should make debugging, etc., easier. We could tone this down a little bit once things are more stable if it's a little too chatty.
    
    Test Plan:
    ```
    $ echo D >> record && git commit -am D && git push
    [master ca5efff] D
     1 file changed, 1 insertion(+)
    # Push received by "local001.phacility.net", forwarding to cluster host.
    # Waiting up to 120 second(s) for a cluster write lock...
    # Acquired write lock immediately.
    # Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
    # Acquired read lock immediately.
    # Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
    # Ready to receive on cluster host "local001.phacility.net".
    Counting objects: 3, done.
    Delta compression using up to 8 threads.
    Compressing objects: 100% (2/2), done.
    Writing objects: 100% (3/3), 256 bytes | 0 bytes/s, done.
    Total 3 (delta 1), reused 0 (delta 0)
    To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
       8616189..ca5efff  master -> master
    ```
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10860
    
    Differential Revision: https://secure.phabricator.com/D15791
  5. @epriestley

    Move all cluster locking logic to a separate class

    epriestley committed Apr 24, 2016
    Summary: Ref T10860. This doesn't change anything, it just separates all this stuff out of `PhabricatorRepository` since I'm planning to add a bit more state to it and it's already pretty big and fairly separable.
    
    Test Plan: Pulled, pushed, browsed Diffusion.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10860
    
    Differential Revision: https://secure.phabricator.com/D15790
  6. @epriestley

    Fix two issues with Remarkup in Pholio

    epriestley committed Apr 25, 2016
    Summary:
    Fixes T10865.
    
      - Mock descriptions did not markup.
      - Image descriptions did not get a proper container `<div />`.
    
    Test Plan:
      - Created a mock with remarkup in the mock description and in an image description.
      - Viewed mock detail.
      - Saw list styles render properly in both mock description and image description.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10865
    
    Differential Revision: https://secure.phabricator.com/D15793
  7. @epriestley

    Include directory-ownership note in `sshd` setup instructions

    epriestley committed Apr 25, 2016
    Summary: Fixes T9560. We suggest a root-owned location, but users who choose their own location instead can run into trouble.
    
    Test Plan:
      - Changed parent directory to have an non-root owner, verified that `ssh` no longer worked.
      - Changed parent directory back to a root owner, verified `ssh` worked again.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T9560
    
    Differential Revision: https://secure.phabricator.com/D15794
Commits on Apr 24, 2016
  1. @epriestley

    Fix bad variable causing `aphlict` to fail to start with no "logs" co…

    epriestley committed Apr 24, 2016
    …nfig
    
    Summary: Fixes T10863. See that task for discussion.
    
    Test Plan:
      - Configured `aphlict` with no "logs".
      - Started `aphlict`.
        - Before change: exception.
        - After change: worked.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10863
    
    Differential Revision: https://secure.phabricator.com/D15788
  2. @epriestley

    Correct Aphlict documentation for Nginx proxying

    epriestley committed Apr 23, 2016
    Summary: Fixes T10857. This documentation did not accurately reflect proper configuration: in the Aphlict config, SSL is inferred from the presence of `ssl.*` configuration.
    
    Test Plan: Read documentation.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T10857
    
    Differential Revision: https://secure.phabricator.com/D15787
Commits on Apr 22, 2016
  1. @epriestley

    Don't try to synchronize repositories with no working copy

    epriestley committed Apr 22, 2016
    Summary:
    Ref T4292. Sometimes, we may not have a working copy for a repository. The easiest way to get into this condition is to deactivate a repository.
    
    We could try to clone + fetch in this case, but that's kind of complex, and there's an easy command that administrators can run manually. For now, just tell them to do that.
    
    This affects the inactive repositories on `secure`, like rGITCOINS.
    
    Test Plan: Removed working copy, got message.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T4292
    
    Differential Revision: https://secure.phabricator.com/D15786
  2. @epriestley

    Improve consistency of file access policies, particularly for LFS

    epriestley committed Apr 22, 2016
    Summary:
    Ref T7789. Currently, we use different viewers if you have `security.alternate-file-domain` configured vs if you do not.
    
    This is largely residual from the days of one-time-tokens, and can cause messy configuration-dependent bugs like the one in T7789#172057.
    
    Instead, always use the omnipotent viewer. Knowledge of the secret key alone is sufficient to access a file.
    
    Test Plan:
      - Disabled `security.alternate-file-domain`.
      - Reproduced an issue similar to the one described on T7789.
      - Applied change.
      - Clean LFS interaction.
    
    Reviewers: chad
    
    Reviewed By: chad
    
    Maniphest Tasks: T7789
    
    Differential Revision: https://secure.phabricator.com/D15784
Something went wrong with that request. Please try again.