1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-15 01:50:55 +01:00
Commit graph

17695 commits

Author SHA1 Message Date
epriestley
2188473fa7 Fix an issue where we may "min()" an empty array when viewing a revision with no changesets
Summary:
Ref T13667. When a revision's diff has no changesets (usually because Diffusion performed an automatic update with an empty commit), the UI currently tries to "min()" an empty array and fatals.

Handle this case properly.

Test Plan:
  - Created a revision with a diff with no changesets ("git commit --allow-empty" + copy-paste into web UI).
  - Viewed revision.
    - Before: "min()" fatal.
    - After: UI isn't perfect, but works without fataling.

Subscribers: cspeckmim

Maniphest Tasks: T13667

Differential Revision: https://secure.phabricator.com/D21760
2022-04-20 13:03:59 -07:00
epriestley
52df4ff515 Fix an issue where "git" may be unable to read a temporary file in Diffusion
Summary:
Ref T13673. After the changes in that task, we may execute "git config -l ..." as a user other than the user we used to write this temporary file.

Use "--file -" to pass the data instead, avoiding use of temporary files. This makes us agnostic to filesystem permissions.

Test Plan: Viewed a Git repository with submodules in Diffusion with "ssh.user" configured as a user relatively isolated from the webserver user.

Maniphest Tasks: T13673

Differential Revision: https://secure.phabricator.com/D21759
2022-04-20 13:03:59 -07:00
epriestley
21b1a304b6 Improve some PHP 8.1 behavior in "bin/auth recover"
Summary: Ref T13588. Fix a couple of argument parsing issues here.

Test Plan: Ran "bin/auth recover" under PHP 8.1.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21757
2022-04-19 14:55:19 -07:00
epriestley
3125d7a5f4 In Git, always "sudo" to the daemon user if a daemon user is configured
Summary:
See T13673. Recent versions of Git (and older versions with backported security patches) now refuse to run Git commands if the top-level repository directory is not owned by the user running the command.

Currently, we "sudo" to that user only when performing writes, so upgrading Git can aggressively break a Phabricator system by knocking out essentially all Diffusion/Conduit read pathways.

As an immediate mitigation, just "sudo" in all cases where a daemon user is available. This fixes the problem, and seems like the least-bad approach. The downside is that the web user may theoretically have fewer privileges than the daemon user and this could reduce the number of layers an attacker armed with some other Git vulnerability might have to get through to do something dangerous (e.g., perform a write on a pathway where only reads are expected), but any separation between the web and daemon accounts is essentially theoretical and has never been enforced.

Test Plan: Applied patch to impacted Phacility shard, saw Diffusion work properly again.

Differential Revision: https://secure.phabricator.com/D21756
2022-04-13 11:23:55 -07:00
epriestley
4dae3e7e1f Improve some UI/language for Phame posts when viewer doesn't have CAN_INTERACT
Summary: Ref T13661. Fix up some UI and language so it's more clear that this is about disabling blog comments.

Test Plan: Viewed UIs, saw some more usable strings.

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21755
2022-04-01 12:52:58 -07:00
epriestley
aae23f0204 Give Phame blog posts configurable interact policies, with a default policy of "Same as Blog"
Summary: Ref T13661. This allows posts to have comments disabled (or restricted) on a per-post basis, and makes them inherit the containing blog policy by default.

Test Plan: Locked a post by editing its policy explicitly; locked a post by editing the containing blog policy.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21754
2022-04-01 12:52:58 -07:00
epriestley
f555fbcb50 Remove unused "MARKUP_FIELD_SUMMARY" for Phame posts
Summary: Ref T13661. This ancient "RemarkupInterface" constant has no references.

Test Plan: Grepped for references, found none. Viewed a blog, viewed a post.

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21753
2022-04-01 12:52:57 -07:00
epriestley
a9822a37aa Fix a PHP 8.1 unit test failure in Projects
Summary: Ref T13588. This field may be "null" (and is probably never the empty string, but that's a more ambitious fix).

Test Plan: Ran unit tests, got a pass.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21752
2022-04-01 12:52:57 -07:00
epriestley
c25595417f Give Phame blogs mutable interact policies
Summary:
Ref T13661.

I'm fairly sure these policies don't actually do anything (you can't "interact" with a blog) but the primarily support a Phame Post object policy of "Same as Parent Blog", which is the "natural" interact policy for a post.

Most of this is infrastructure support for mutable interact policies: today, only Maniphest has interact mutability and only via indirect effects (locking tasks), not through a directly mutable "Can Interact" policy.

Test Plan:
Ran storage upgrade, edited interact policy of a blog, saw appropriate persistence and transactions.

Created and edited a task to make sure there's no weird fallout from increasing what can be done with interact policies.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21751
2022-04-01 12:52:56 -07:00
epriestley
5ce7f803f9 Fix double-bordered breadcrumbs in Phame blogs
Summary: Ref T13661. The Phame blog main page has a double breadcrumb border in non-live mode.

Test Plan: Viewed a blog page in internal and live modes, saw single border in both cases (previously: double border for internal mode).

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21750
2022-04-01 12:52:56 -07:00
epriestley
6c2c80a373 Remove ancient Remarkup constants from Phame and Maniphest
Summary: Ref T13661. These constants are no longer used.

Test Plan:
For a blog with a nonempty description, viewed the "Manage" page and the home page; saw properly rendered description.

Grepped for removed constants, no hits.

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21749
2022-04-01 12:52:55 -07:00
epriestley
28b12fb3cd Make Phame blog policies non-nullable
Summary: Ref T6203. Ref T13661. These policies are incorrectly nullable, although it's likely that no pathway exists in the application to write NULL into them. Fix the schema.

Test Plan: Ran `bin/storage upgrade`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661, T6203

Differential Revision: https://secure.phabricator.com/D21748
2022-04-01 12:52:55 -07:00
Matthew Bowker
7d4357683a Hide the blurb of a user when that user is disabled
Summary: T15074

Test Plan: Disabled a user, then made sure their blurb disappeared.

Reviewers: O1 Blessed Committers, avivey, speck

Reviewed By: O1 Blessed Committers, avivey, speck

Subscribers: 20after4, avivey, speck, tobiaswiese, valerio.bozzolan

Maniphest Tasks: T15074

Differential Revision: https://we.phorge.it/D25035
2022-03-30 09:17:29 -06:00
epriestley
dec95228a2 Add a "--database <name> ..." flag to "bin/storage dump"
Summary: Ref T13671. Allow "bin/storage dump" to dump a subset of databases, primarily to support merging previously-partitioned databases.

Test Plan: Ran `bin/storage dump` with and without `--database ...` flags. Ran `--database invalid`, `--database a --database a` to hit error cases.

Maniphest Tasks: T13671

Differential Revision: https://secure.phabricator.com/D21745
2021-12-18 11:11:09 -08:00
epriestley
6136964093 Fix a PHP 8.1 strlen() issue with "mysql.pass" configuration
Summary:
Ref T13588. This configuration value may not be set.

Also fix an issue in `bin/storage` and whatever else I hit between now and this diff actually uploading.

Also fix a MySQLi report mode difference, beginning in PHP 8.1.

Also update a bunch of "static" property usage in Lisk.

Test Plan: Ran `bin/files ...` locally under PHP 8.1.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21744
2021-12-16 15:24:21 -08:00
epriestley
dc705cea7f Document Mailgun as discouraged, and update Postmark remote address blocks
Summary:
Ref T13669. See that task for discussion. Based on a disheartening interaction in June 2020, discourage use of Mailgun.

Also update the default Postmark inbound address list, since they've added a new address.

Test Plan: Read documentation, grepped for "mailgun".

Maniphest Tasks: T13669

Differential Revision: https://secure.phabricator.com/D21738
2021-12-02 14:56:36 -08:00
epriestley
8103d36177 Remove "bin/celerity" dependency on database setup
Summary: Ref PHI2157. Like other low-level tools, "bin/celerity" does not need databases configured in order to execute.

Test Plan: Ran `bin/celerity map` with and without the database available.

Differential Revision: https://secure.phabricator.com/D21730
2021-11-19 10:11:06 -08:00
epriestley
c7b04170fe Provide missing "AlmanacInterfaceTransactionQuery"
Summary:
Ref PHI2149. This provides the trivial Query class required for the daemons to publish Almanac Interface transactions.

(Publishing these doesn't do anything interesting, but currently leaves an error in the daemon logs.)

Test Plan:
  - Stopped the daemons.
  - Edited the port of an Interface in Almanac.
  - Ran `bin/worker execute --active --class PhabricatorApplicationTransactionPublishWorker` to publish the transaction.
    - Before: fatal on missing class, "Unable to load query for transaction object...".
    - After: transaction publishes cleanly.

Differential Revision: https://secure.phabricator.com/D21726
2021-11-16 11:48:25 -08:00
Leon Eckardt
b293e6ffed Make Datepicker translatable
Summary: This adds the Ablillity to translate the Content of the Date Picker. It is possible to display more than one Letter for the Day Captions.

Test Plan:
 - Change the Language in the User Preferences, make sure it translates 'S|M|T|W|T|F|S' and Months or add the following to the translation.override config to get German Translation:

```
{
    'S|M|T|W|T|F|S': 'So|Mo|Di|Mi|Do|Fr|Sa',
    'January': 'Januar',
    'February': 'Februar',
    'March': 'März',
    'May': 'Mai',
    'June': 'Juni',
    'July': 'Juli',
    'October': 'Oktober',
    'December': 'Dezember'
}
```
 - Open Datepicker on a Datefield (Edit Event or Edit for Custom Field of Type date)
 - The Day Headers and the Months should now be translated

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: CSharp, 0, speck, tobiaswiese

Tags: #calendar

Differential Revision: https://we.phorge.it/D25016
2021-09-21 20:45:32 -04:00
epriestley
b9ba0418ed Correct a parameter order swap in "diffusion.historyquery" for Mercurial
Summary: Ref T13666. See T13666#256253. The order of these parameters is incorrect (introduced in D18817, which was trying to turn the revset "A or B" into "B or A"), but when "commit" is the single head of "branch" (which is common), the revset is functionally equivalent.

Test Plan:
An easy reproduction case is to make a "diffusion.historyquery" call manually against a Mercurial repository, with a valid "branch" name and some non-head "commit".

  - Before patch, queried "head^" (by commit hash, not by symbol) of "default" and got "head" too (wrong).
  - Applied patch to `secure001.phacility.net` (my local `hg` web environment is iffy right now).
  - Re-ran the same query, saw "head^" as the first result rather than "head" (all hashes rather than symbols, as above), which is desired.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Maniphest Tasks: T13666

Differential Revision: https://secure.phabricator.com/D21722
2021-09-04 18:16:32 -07:00
Christopher Speck
3b2868e155 Fix searching legalpad documents by contributors
Summary:
Found in a change submitted to Phorge (https://we.phorge.it/D25018#inline-231), this fixes a typo in populating search the `contributorPHIDs` constraint in the Legalpad search engine.

Currently when trying to search legalpad documents by contributor an error is encountered:
```lang=console
Array for %Ls conversion is empty. Query: contributor.dst IN (%Ls)
```

Test Plan: I searched for legalpad documents based on a contributor and got back correct results.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D21719
2021-09-04 20:37:29 -04:00
Christopher Speck
09c3c7d879 Add support to marking commits as UNREACHABLE for Mercurial
Summary:
When previously known commits have been destroyed in a Mercurial repository, Phabricator does not end up marking the commits as unreachable. This results in daemon tasks which continuously fail and retry.

This updates `PhabricatorRepositoryDiscoveryEngine` and `PhabricatorManagementRepositoryMarkReachableWorkflow` to include support of marking commits as unreachable for Mercurial repositories.

The `PhabricatorMercurialGraphStream` also needed updated to support a stream with no starting commit.

Refs T13634

Test Plan:
1. I set up a hosted Mercurial repository.
2. I removed the head commit from the on-disk repository state.
3. I attempted to load the repository page and saw an exception due to a missing commit.
4. I went to `/manage` for the repository and scheduled an update of the repository.
5. After an updated performed, I went to the repository main page and saw there was no exception and the history view properly did not have the commit I had removed.
6. I checked the phd logs and verified there were no exceptions related to the repository.
7. I ran the `./bin/repository mark-reachable` command on the Mercurial repository and it reported that it marked the commit as unreachable.
8. I pushed the same commit back upstream and verified that the commit was found and displayed in the history view of the repository page and `mark-unreachable` did not identify it as being unreachable.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13634

Differential Revision: https://secure.phabricator.com/D21715
2021-09-04 19:05:08 -04:00
Christopher Speck
458ad4a861 Remove history query from DiffusionRepositoryController as it is unused
Summary:
The history query for the repository page isn't actually used to display any content. It looks like it was previously used to display the last user which modified a file however this looks to be removed in D21404. This removes the history query from happening as well as updates `DiffusionBrowseTableView` to remove the parameters for passing this information in, resulting in also updating `DiffusionBrowseController` to no longer need to put this information together.

Refs T13666

Test Plan:
1. I removed commits from a repository on the local state.
2. I navigated to the repository's landing page and saw that the landing page attempted to render content and only failed to load the browse files section.
3. I navigated to the history tab and verified that it showed an exception about failing to query commit information.
4. I restored the repository working state to function properly.
5. I navigated to a repository's landing page and verified it loaded properly, including showing the last modified date for each file.
6. I navigated to the Code, Branches, Tags, and History tabs to verify each tab page loaded properly.
7. I verified on the Code tab that the last modified date for each file displayed properly.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13666

Differential Revision: https://secure.phabricator.com/D21717
2021-09-04 18:50:46 -04:00
epriestley
b757e5c302 Use "MethodCallFuture" to move Diffusion Conduit exceptions to resolution time
Summary:
Depends on D21720. Ref T13666. See D21720 for additional discussion.

Use "MethodCallFuture", introduced in D21720, so that exceptions raised in "execute()" are thrown when the future is resolved, not when the future is created.

This makes exception behavior for clustered and non-clustered setups consistent, and chooses the intended (clustered) behavior in both cases, which currently deals with errors better.

Test Plan:
  - Applied both parts of the patch in T13666 (break history queries, force immediate futures) to reproduce the issue in T13666.
    - Loaded a Diffusion landing page, reproduced the error described in that task.
  - Applied this patch, verified landing page works again.
  - Removed the "break history queries" change, verified landing page works in forced-immediate mode.
  - Removed the "force immediate" change, verified landing page works in "actual future" mode.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Maniphest Tasks: T13666

Differential Revision: https://secure.phabricator.com/D21721
2021-09-04 14:36:41 -07:00
Leon Eckardt
fd1922c763 Update Install and Update Scripts to the new Paths and Repository links
Summary:
Set the Reposity links to actually install Phorge, Update Documentation links to the new Website.

Notes:
 - Github Mirrors are not set up. Use one anyway? Or just use the Repos at we.phorge.it?
 - Documentation Links still contain "phabricator". The Docs are changed, but the Diviner Books are not rebuilt. (Add a Cronjob or do it after every Commit?)
 - mysql-server is not included anymore in Debian environments. Should mariadb-server be used?
 - Should we print somewhere that Phorge is a Fork of Phabricator?

Test Plan:
Run the install script.
It should install Phorge and display Links to we.phore.it

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese

Differential Revision: https://we.phorge.it/D25019
2021-09-04 15:48:49 -04:00
MacFan4000
ce31289228 (PhabricatorENV) update doclinks to link to we.phorge.it instead of secure.phabricator.com
Summary: With this change, links to documentation will point to we.phorge.it rather then secure.phabricator,com

Test Plan: No testing needed - simple URL update

Reviewers: O1 Blessed Committers, deadalnix

Reviewed By: O1 Blessed Committers, deadalnix

Subscribers: taavi, speck, tobiaswiese

Differential Revision: https://we.phorge.it/D25014
2021-09-04 15:47:05 -04:00
James Brown
a4948ec800 Add conduit endpoints for querying legalpad
Summary:
This diff adds conduit methods for searching for legalpad documents and signatures. This is very helpful for auditing who's actually signed a document. It also fixes the "contributorPHIDs" constraint in the existing search engine.

In order to expose legalpad signatures through Conduit, this adds a `phid` column to the `legalpad_documentsignature` table. It includes a migration (in the style of many previous phid-adding migrations) to actually populate the column.

Test Plan: We run this on my company's internal fork and it seems to work okay. I don't think any other conduit methods anywhere have tests (???), but if you can point me at one I'm glad to write a unit test!

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: 20after4, speck, tobiaswiese

Differential Revision: https://we.phorge.it/D25018
2021-09-04 13:53:13 -04:00
Dmitri Iouchtchenko
1965b78b34 Fix subtype extension support check
Summary: Fixes T13663. `supportsSubtypes` tries to create an editable object, but this isn't always valid for `PhabricatorCalendarImport`. Use `instanceof` instead.

Test Plan:
  - Edited calendar import, tasks (2 different subtypes), and projects (2 different subtypes).
  - Changed task subtypes using {nav Change Subtype} action and batch editor.
  - Changed task and project subtypes using Conduit.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T13663

Differential Revision: https://secure.phabricator.com/D21714
2021-08-20 18:49:35 -04:00
epriestley
8daaf5ef21 Pass a real context object to Phriction previews, fixing mentions
Summary:
Fixes T13662. Phriction currently passes a map as a "context object", but this code is ancient and predates the modern meaning of a "context object". In modern code, context objects should be real objects.

Provide a real object as a context object. We do this by either loading the actual document or constructing a synthetic version of it.

Test Plan:
  - Edited an existing document, observing the preview:
    - Used a mention rule, saw a preview.
    - Used `[[ a ]]` and `[[ ./a ]]` absolute and relative reference rules, saw accurate previews.
  - Edited a new document, observing the preview:
    - Used a mention rule, saw a preview.
    - Used absolute/relative references, saw accurate previews.
  - Grepped for other references to the removed properties (`phriction.isPreview` and `phriction.slug`), found none remaining.

Reviewers: 0

Reviewed By: 0

Maniphest Tasks: T13662

Differential Revision: https://secure.phabricator.com/D21709
2021-08-02 08:36:15 -07:00
epriestley
c7550dbee9 Fix Phriction document previews for the root document ("/") with Apache option "MergeSlashes On"
Summary:
Ref T13662. I ran into this while trying to reproduce the mention issue discussed there.

Currently, the root document (with slug "/") attempts to preview using the URI `/phriction/preview//` (with two `//` at the end). This is collapsed into "/phriction/preview/" by Apache if "MergeSlashes On" is configured, which is the default behavior. The route then 404s.

Instead, just use "/phriction/preview/?slug=/" so this endpoint functions properly regardless of the "MergeSlashes" configuration.

Test Plan:
  - Configured Apache with "MergeSlashes On" (which is the default behavior).
  - Tried to preview a content edit of the root document in Phriction, which didn't work and generated 404s for "/phriction/preview//" in the console log.
  - Applied patch.
  - Previwed content in Phriction (which now worked properly).
  - Accessed `/a//b///c////` and similar with "MergeSlashes On" and "MergeSlashes Off", confirmed that this option controls whether PHP receives a URI with or without merged slashes in "__path__" after rewriting.

Reviewers: 0

Reviewed By: 0

Maniphest Tasks: T13662

Differential Revision: https://secure.phabricator.com/D21708
2021-08-02 08:36:15 -07:00
epriestley
dbe2fb466f Add an "eval" rule to Remarkup
Summary:
Ref T13658. This adds a simple expression evaluator to Remarkup and supports platform name expressions. The syntax is:

```
${{{strings.platform.server.name}}}
```

Note that this won't work inside code blocks (or literal blocks, or other block-level literal elements) right now, although it could be made to selectively (the ".path" expressions might be useful in documentation codeblocks).

Test Plan: {F9391006}

Reviewers: cspeckmim

Reviewed By: cspeckmim

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21713
2021-07-29 16:46:50 -07:00
epriestley
cc8cbed029 Make "DifferentialDiff->properties" a proper "attachable" property
Summary:
See PHI498. This should be initialized to "self::ATTACHABLE" like other attachable properties, but is currently initialized to "array()".

Initialize it the normal way and try to catch all code paths which may have accessed it without actually loading and attaching it.

Also, remove UI for the very old "excuse" property, which "arc" has not written for well more than a year.

Test Plan: Grepped for affected symbols, loaded various revision pages. Somewhat tricky to be 100% sure that every pathway is caught, but it should be obvious if I missed anything once someone hits the code path.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D21710
2021-07-27 13:39:12 -07:00
epriestley
387d3b4983 Resolve deleted packages properly as having no mailable members
Summary:
Fixes T13648. If a package recipient has been destroyed, this query currently fails to return an expanded recipient value.

Instead, make sure all input PHIDs get an output. For destroyed packages, the output will just be an empty list.

Test Plan:
  - Added a package to a revision as a reviewer.
  - Destroyed the package.
  - Commented on the revision.
  - Processed the publishing worker with `bin/worker execute`.
  - Before: fatal after expanding the destroyed package.
  - After: clean publish.

Maniphest Tasks: T13648

Differential Revision: https://secure.phabricator.com/D21707
2021-07-21 14:50:07 -07:00
epriestley
4cffaa600b Rename "HarbormasterRestartException" to "HarbormasterMessageException"
Summary: Ref T13072. This exception is now raised by all of the message-sending code. Pretty straight find/replace.

Test Plan: Grepped for old class name, no hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21699
2021-07-21 14:17:00 -07:00
epriestley
4c4123f98b Allow "harbormaster.sendmessage" to send control command (pause, restart, abort, resume) to Builds/Buildables
Summary: Ref T13072. Expand the role of "harbormaster.sendmessage" and allow it to send control messages to Builds and Buildables.

Test Plan: Read documentation, sent commands to Builds and Buildables, hit a bunch of error cases, will deploy to catch full-lifecycle Build Target use cases.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21698
2021-07-21 14:17:00 -07:00
epriestley
3df1e17527 Add a side nav to Conduit API method console pages
Summary: Ref T13072. Make large Conduit doc pages a bit more navigable. This prepares for updating "harbormaster.sendmessage" to support sending messages to builds.

Test Plan: Viewed various Conduit API documentation pages, clicked links.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21696
2021-07-21 14:16:59 -07:00
epriestley
2ff1d4b3b0 Add stub "harbormaster.build.edit" and "harbormaster.buildable.edit" API methods
Summary: Ref T13072. These don't do anything useful yet, but get the skeletons in.

Test Plan: Loaded documentation pages without fataling.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21695
2021-07-21 14:16:59 -07:00
epriestley
346ffc51e1 Modularize "HarbormasterBuildableTransaction"
Summary: Ref T13072. Trivially convert this into a modular transaction type.

Test Plan: Issued commands to a buildable.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21694
2021-07-21 14:16:58 -07:00
epriestley
706b91adf6 Remove "HarbormasterBuildableTransaction::TYPE_CREATE"
Summary: Ref T13072. This transaction type has no writers and is mooted by EditEngine.

Test Plan: Grepped for transaction constant.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21693
2021-07-21 14:16:57 -07:00
epriestley
1a1395579f Remove "HarbormasterBuildCommand"
Summary: Ref T13072. Update the last few constant references to this class and remove it.

Test Plan: Grepped for "HarbormasterBuildCommand", got no hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21692
2021-07-21 14:16:57 -07:00
epriestley
1212dc5fbe Modularize almost all Harbormaster build message workflows and UI/UX
Summary: Ref T13072. Push nearly all Harbormaster build message logic into the new per-message transaction classes.

Test Plan:
  - Issued every message to Buildables.
  - Issued every message to Builds.
  - Looked at a big pile of error messages, couldn't find any typos.
  - Grepped for affected symbols, etc.
  - Ran `bin/harbormaster restart ...`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21691
2021-07-21 14:16:56 -07:00
epriestley
8bbee92139 Modularize individual Harbormaster build messages
Summary:
Ref T13072. Further modularize build messages by applying each one in a separate transaction type.

This makes it easier to add new types of messages (although I have no particular plans to do this, offhand) and reduces the amount of switch-boilerplate.

This will probably also simplify validating "harbormaster.sendmessage".

Test Plan:
  - Applied all commands.
  - Ran migration, saw transactions render properly

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21690
2021-07-21 14:16:56 -07:00
epriestley
6dfea0adad Modularize HarbormasterBuildTransaction
Summary: Ref T13072. Update "HarbormasterBuild" to use modern modular transactions.

Test Plan:
  - Aborted, restarted, paused, and resumed a build.
  - Used `bin/harbormaster restart`.
  - Grepped for use of old "::TYPE_COMMAND" constant, didn't find any hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21689
2021-07-21 14:16:55 -07:00
epriestley
b84c02959a Remove "HarbormasterBuildTransaction::TYPE_CREATE"
Summary: Ref T13072. No callers currently generate these transactions, and they probably never have. Remove them.

Test Plan: Grepped for "HarbormasterBuildTransaction::TYPE_CREATE" and "self::TYPE_CREATE" in the class, found no hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21688
2021-07-21 14:16:55 -07:00
epriestley
bf83fffca1 Correct the flow of edit authority when sending messages to HarbormasterBuild objects
Summary:
Ref T13072. Currently, Harbormaster builds react to messages by applying a transaction inline (which can race) that has no real effect.

Later, the BuildEngine picks up the mesasge and applies a real effect, but this isn't transactional.

This is backwards, and makes it more difficult to transition to ModularTransaction and EditEngine. The desired workflow is:

  - sending a message //just// writes to the message table (and queues a worker to process the message);
  - the BuildEngine processes the message and applies effects in a transactional way.

Force this into at least roughly the right sequence of behaviors. This paves the way for porting to ModularTransaction, which should allow further cleanup.

Test Plan: Paused, resumed, aborted, and restarted a build. Ran BuildWorkers to process the commands, saw builds update appropriately.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21687
2021-07-21 14:16:54 -07:00
epriestley
012af00731 Improve formality of "HarbormasterBuild" states
Summary:
Ref T13072. Currently, Builds have basic states (like "passed" and "failed") and pending states where a command has been issued but not yet executed (pausing, resuming, restarting, and aborting).

These are handled in a bit of an ad-hoc way, and not everything treats them the same way. In particular, the build page can concurrently report a build as "Aborting" and "Pausing", with different icons and colors.

Make everything use the same logic so that a Build can only be in exactly zero or one pending state, and use the same icons and colors.

Also tighten up which transitions are allowed: for example, it doesn't make sense to pause an aborting build.

The tighter rules don't all produce great UX right now (like "You can't pause this build.", when it would be better as "You can't pause a build which is already aborting." or similar), but just leave that alone for now.

Test Plan: Viewed builds, applied various state changes, ran BuildWorker to effect the state changes, grepped for affected methods, tried to issue various out-of-sequence build commands.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21685
2021-07-21 14:16:54 -07:00
epriestley
b48d4fabaf Merge the "HarbormasterBuildCommand" table into "HarbormasterBuildMessage"
Summary:
Ref T13072. These two similar tables don't make sense to keep separate. Instead, make Build a valid receiver for BuildMessage objects.

These tables are practically the same, so this is straightforward: just copy the rows in and then drop the old table.

(This table was trivial and ephemeral anyway, so I'm not bothering to do the usual "keep it around for a couple years just in case".)

Test Plan:
  - Populated BuildCommand table, ran migration, saw Builds end up in the proper transitional state (e.g., pausing, aborting, restarting) with appropriate queued messages.
  - Queued new messages by clicking UI buttons.
  - Ran BuildWorkers, saw them process messages and mark them as consumed.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21684
2021-07-21 14:16:53 -07:00
epriestley
0f93d1ffe4 Rename "HarbormasterBuild" methods to prepare for use of the "BuildMessages" table
Summary:
Ref T13072. Rename various "command" properties to "message" properties, to prepare for merging "HarbormasterCommand" into "HarbormasterMessage".

This change only renames variables and methods and should not affect program behavior.

Test Plan: Grepped for affected symbols, found no unmodified hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21683
2021-07-21 14:16:52 -07:00
epriestley
af6cc0c934 Use "resolve()", not "execute()", for PhutilExecPassthru callsites in Phabricator
Summary: Ref T13660. Clean up callsites to "PhutilExecPassthru->execute()" to prepare to deprecate it.

Test Plan:
  - Grepped for "PhutilExecPassthru" and looked for callsites.
  - Ran `GIT_SSH=.../ssh-connect git ls-remote origin` to execute the "ssh-connect" code.
  - The two passthru future methods have no callers and could possibly be removed, but I'm just letting sleeping dogs lie for now.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Maniphest Tasks: T13660

Differential Revision: https://secure.phabricator.com/D21703
2021-07-21 10:21:06 -07:00
Dmitri Iouchtchenko
2f1acf8b10 Suppress PHP 8 deprecation warning in startup
Summary: Ref T13588. See D21497. As of PHP 8, the XML entity loader is disabled by default and the `libxml_disable_entity_loader` function is deprecated. Thus suppress the deprecation warning for now; we could skip the function call, but this is safer.

Test Plan:
* Still works with PHP 7.
* No more deprecation message with PHP 8.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21701
2021-07-20 21:07:33 -04:00