Summary: Make the "version" data in Config show information. Also rename the library.
Test Plan: See version "branched from" information.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew
Maniphest Tasks: T15006
Differential Revision: https://we.phorge.it/D25047
Summary:
Ref T13685. "RemarkupEditField" uses a "RemarkupValue" internally, and it currently attempts to serialize into the database unsuccessfully.
Instead, flatten the value before returning it for storage.
Test Plan:
- Edited the default description of a task in a custom form.
Maniphest Tasks: T13685
Differential Revision: https://secure.phabricator.com/D21854
Summary:
Ref T13588. See PHI2198. Some pathways represent SVN commit identifiers (which are numeric) with integers; these raise an exception in `phutil_nonempty_string()`, which is strict.
(Ideally, commit identifiers wouldn't have mixed types, but that's a dramatically less practical change.)
Test Plan: Will deploy.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21852
Summary: See <https://hackerone.com/reports/1573143>. The pathway for disabling global builtin queries is missing a policy check. Add it.
Test Plan:
- Accessed the "/search/delete/id/.../" URI for a global builtin query as a non-administrator.
- Before patch: could improperly disable queries.
-After patch: proper policy exception.
Differential Revision: https://secure.phabricator.com/D21851
Summary:
Ref T13682. Versioned drafts may have missing or unexpected metadata:
- versioned drafts from an older version of Phabricator may be missing metadata;
- versioned drafts created by an older UI against a newer version of Phabricator may have `null` metadata.
Generally, make these workflows robust to metadata in unexpected formats, so database debris doesn't break the UI.
Test Plan: Simulated debris, interacted with UI.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21849
Summary: Ref T13682. This prepares for modernizing Slowvote and exposing a more usable API.
Test Plan: Ran migrations, opened and closed polls.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21848
Summary: Ref T13682. Prepares for use of API-friendly string constants rather than opaque integers.
Test Plan: Created and edited polls, opening and closing them. Grepped for affected methods.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21847
Summary: Ref T13682. Use API-friendly string constants instead of opaque integers in Slowvote voting methods.
Test Plan: Created, edited, and voted in polls with various voting methods. Examined database after migrations.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21846
Summary: Ref T13682. Extract Slowvote vote types to a separate object, to prepare for turning them into API-friendly string constants.
Test Plan: Created, edited, and voted in Slowvote polls. Grepped for affected constants.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21845
Summary: Ref T13682. Migrate and update Slowvote to use API-friendly string constants for response visibility, not opaque integers.
Test Plan: Created and edited slowvotes, changing response visibility.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21844
Summary: Ref T13682. This change supports modifying these constants to be sensible strings instead of opaque integers.
Test Plan: Created and edited polls.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21843
Summary:
Ref T13682. Ref T13065. Migrate and update "SlowvotePoll" to use more modern mailkey storage (in the mail application, rather than onboard).
Also make other minor modernization changes.
Test Plan: Created a new poll. Ran migration, verified mail key copied properly in database.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682, T13065
Differential Revision: https://secure.phabricator.com/D21842
Summary: Ref T13682. Use a more modern PHID key name for this key.
Test Plan: Ran `bin/storage adjust`, got a modern key.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21841
Summary: Ref T13682. Make the "Attached" list in Files a bit more detailed, and add a "Detach" button.
Test Plan: Tried to detach unrelated, referenced, and attached files. Saw attached files detach.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21840
Summary:
Ref T13682. When editing a comment, the edit pathway may invoke file PHID extraction without first formalizing an object on the Editor.
Pass the "object" explicitly to generally improve the robustness of this pathway.
Test Plan: Edited a comment.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21839
Summary: Ref T13682. Many subclasses of "CursorPagedPolicyAwareQuery" have the same implementation of "loadPage()", and this is a sensible default behavior.
Test Plan: Examined changes to verify that all removed methods have the same behavior.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21838
Summary: Ref T13682. Allow users to manually attach files which are referenced (but not attached) via the UI.
Test Plan: Reference files via `{F...}`, then attached them via the UI workflow.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21837
Summary: Ref T13682. Provide a basic UI element for showing file attached to an object.
Test Plan: Viewed objects with attached files in the UI.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21836
Summary:
Ref T13682. This is a marginal case, but if you quote someone else's text and it has a file reference embedded in it, that isn't good enough to generate "attachment intent".
Since you need both a reference and an explicit attachment, this should never actually affect any user-visible behavior today, but makes the ruleset more thorough.
Test Plan: Dragged and dropped a file, referencing it in either a quoted or unquoted block. Saw it attach only for a quoted block.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21833
Summary:
Ref T13682. When a user uploads a file, then changes their mind and deletes the reference to the file, we don't actually want to attach the file.
When choosing which files to attach, only attach files which are both referenced in Remarkup and explicitly attached in remarkup metadata.
Test Plan:
- Dropped a file into a comment, submitted it, saw it attach normally.
- Dropped a file into a comment, deleted the reference, submitted it, saw no attachment.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21832
Summary: Ref T13682. This property may not exist, and should defualt to an empty array if not present.
Test Plan: Created a new object.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21834
Summary:
Ref T13603. On common edit pathways, extract explicit file attachments from Remarkup. These pathways are affected:
- Objects that use EditEngine and expose a remarkup area via "RemarkupEditField".
- Objects that use EditEngine to generate a comment area.
This is the vast majority of pathways, but not entirely exhaustive.
Test Plan: Created and commented on a task, explicitly attaching images. Saw images attach properly.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21830
Summary:
Ref T13603. This removes the file attachment edge types, which no longer have readers or writers.
It does not delete the underlying data.
This indirectly removes API access to this edge. As far as I know, this was only used by one customer (see D21480) who has migrated away from Phabricator.
Test Plan: Grepped for edge constants.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21823
Summary: Ref T13603. Removes all reads and writes to the other half of the legacy edge.
Test Plan:
- Verified I could still see file content accessible only through object attachment.
- This fixes a unit test broken by removal of only half the edge access in the previous change.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21820
Summary: Ref T13603. Migrate all code which interacts with the "ObjectHasFile" edge to use the "Attachments" table instead.
Test Plan:
- Edited a paste's view policy, confirmed associated file secret was scrambled.
- Verified I could still view paste content as a user who could not naturally view the underlying file.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21819
Summary: Ref T13603. New transactional file attachment extracts PHIDs earlier than the older mechanism did. Allow either pathway to save content and extract a PHID.
Test Plan:
- Created and modified a paste.
- Confirmed both pathways do old and new storage writes.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21817
Summary: Ref T13603. This adds a second write to new "attachment" storage to all writers except one in Paste, which creates the file inline.
Test Plan:
- Updated a macro image, confirmed a write to "attachment" storage (transaction pathway).
- Updated a blog profile image, confirmed a write to "attachment" storage (legacy pathway).
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21816
Summary:
Ref T13603. Currently, files are sometimes detached from objects. For example, when you change the image for a Macro, the old image is detached.
This is wrong: the image should remain attached so users who can view the macro can view the complete "alice change the image from X to Y" transaction. To be able to understand the change that was applied, you need to be able to view both files.
All workflows which currently detach files aren't conistent with the modern way applications behave, except maybe one callsite in a unit test, and that one's kind of moot.
Get rid of this stuff and just use PHID extraction to perform file attachment in all cases.
Test Plan: Created and edited macros, verified files were properly attached and remained attached across edits.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21815
Summary: Ref T13603. Prepare to move this relationship out of edge storage into dedicated storage so it is easier to formalize better in the UI.
Test Plan: Ran `bin/storage upgrade`.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21813
Summary: Ref T13603. When processing transactions that include remarkup blocks, stop extracting file references from them for attachment.
Test Plan: Submitted a remarkup block with a file reference, no longer saw automatic file attachment.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21812
Summary: Ref T13588. This allows "arc unit --everything" to more or less run cleanly on PHP 8.1.
Test Plan: Ran "arc unit --everything".
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21821
Summary:
Ref T13603. Allow "VersionedDraft" to persist remarkup comment area metadata from stacked actions controls.
When files are dragged and dropped, record them as explicit uploads in comment metadata.
Test Plan: Dragged and dropped files into Remarkup stacked action text areas (e.g., in Maniphest), reloaded page, saw metadata persist across reloads.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21828
Summary:
Ref T13681. This was introduced in D593, never used, and doesn't make sense or have any uses in modern Phabricator.
It also does some pretty direct writes that can technically do things that at least //look// like they violate policies, so remove it.
Test Plan:
- Checked the API console, no longer saw "feed.publish".
- Grepped for "feed.publish", no hits.
Maniphest Tasks: T13681
Differential Revision: https://secure.phabricator.com/D21826
Summary:
Ref T13679. In D16983, global settings objects were given an exception to let logged-out users see them, even on installs with no "public" user role.
This exception is too broad and grants everyone all capabilities, not just "CAN_VIEW". In particular, it incorrectly grants "CAN_EDIT", so any user can edit global settings defaults.
Restrict this grant to "CAN_VIEW".
Test Plan:
- As a non-administrator, tried to edit global settings.
- Before: could.
- After: could not.
Maniphest Tasks: T13679
Differential Revision: https://secure.phabricator.com/D21811
Summary: Ref T13677. This was an accidental change in D21807: when reclaiming a resource, wait for it to be completely destroyed before allowing a lease to reclaim another resource.
Test Plan: Reverts accidental behavior change in D21807.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13677
Differential Revision: https://secure.phabricator.com/D21809
Summary:
Ref T13677. Drydock has a hard-coded and fairly arbitrary limit which prevents a resource pool from growing more than 25% at once.
This is vaguely reasonable for resources which allocate quickly, but suffocating for slower resources. It's also wholly arbitrary, and the "one per lease" limit introduced in D21807 should do a better job of covering the same ground while generally being more reasonable, expected, and predictable.
Test Plan: Ran Drydock allocations without the throttle, saw faster pool growth.
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13677
Differential Revision: https://secure.phabricator.com/D21808
Summary:
Ref T13677. Currently, one lease may cause multiple resources to allocate simultaneously if it starts allocating one, then wakes up from a yield later on and still sees no available resources.
This is never desired -- or, at least, produces desirable behavior only entirely by accident. Normally, it causes an excess of resources to allocate.
This is not a catastrophic problem: the extra resources usually get used sooner or later or cleaned up; and the total amount of badness is limited by overall resource allocation limits.
However, this behavior is also suppressed by an artificial "25% of current pool size" growth limit throttle which I intend to remove. Removing this throttle without fixing the allocator behavior could make this "too many resources" problem worse.
Change the allocator so that a lease that has started allocating a resource won't allocate another resource until the first resource leaves the "pending" state.
This also fixes some general oddness with the allocator and attempts to simplify the structure.
Test Plan:
- Ran 8 taskmasters.
- Destroyed all resources and leases.
- Leased 4 working copies.
- Saw exactly 4 resources build and lease, all simultaneously.
- Destroyed all resources and leases.
- Leased 32 working copies.
- Saw exactly 32 resources build and lease, approximately 8 at a time (limited by taskmasters).
- Destroyed all leases (but not resources).
- Leased 32 working copies, saw them satisfied by existing resources.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13677
Differential Revision: https://secure.phabricator.com/D21807
Summary:
Ref T13677. Track which resources a given lease has begun allocating or reclaiming in a more formal way, and add logging for waiting actions.
The "allocating" mechanism is new. This will replace an existing similar "reclaiming" mechanism in a future change.
Test Plan: See followup changes.
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13677
Differential Revision: https://secure.phabricator.com/D21806
Summary: Ref T13677. These flags increase the convenience of testing in a development environment.
Test Plan: Used new "--all" flags to release all resources and leases.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13677
Differential Revision: https://secure.phabricator.com/D21805
Summary:
Ref T13676. The 3-minute grace period when a resource can not be reclaimed after its leases are released currently doesn't work reliably because the Resource object usually isn't actually updated when a lease is released.
Add an additional check for recently-destroyed leases, and extend the grace period if we find any.
Test Plan:
- See T13676. Ran reproduction sequence there, observed immediate resource reclamation.
- Applied patch.
- Ran sequence again, observed repository B wait 3 minutes to reclaim a repository A resource.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676
Differential Revision: https://secure.phabricator.com/D21803
Summary:
Ref T13676. Currently, "bin/drydock lease" just creates a lease that permits any blueprint.
To prepare for "use specific blueprint X", unify the logic between this workflow and LeaseUpdateWorker so we select only blueprints which at least have coarse compatibility (e.g., if we're leasing a host, only select enabled blueprints of classes that can allocate hosts).
Test Plan: Used `bin/drydock lease` to try to lease a nonsense type, got sensible error. Leased a host.
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676
Differential Revision: https://secure.phabricator.com/D21801
Summary: Ref T13676. This makes it easier to create resource pressure without juggling a big pile of terminals.
Test Plan: Used `bin/drydock lease --count 5 ...` to acquire 5 leases.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676
Differential Revision: https://secure.phabricator.com/D21800
Summary: Ref T13676. When the required "repositories.map" attribute is omitted, `bin/drydock lease` currently fatals in an unhelpful way when trying to lease a working copy.
Test Plan:
Ran `bin/drydock lease --type working-copy` with no attributes, after following steps in T13676.
```
<Allocation Failed> One or more blueprints promised a new resource, but failed when allocating: [PhutilAggregateException] All blueprints failed to allocate a suitable new resource when trying to allocate lease ("PHID-DRYL-orbtwtlinksm3xqpyhmw").
- Exception: Working copy lease is missing required attribute "repositories.map".
Attribute "repositories.map" should be a map of repository specifications.
```
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676
Differential Revision: https://secure.phabricator.com/D21796
Summary: Ref T13676. Ref T13588. Fix some issues that prevent "bin/phd" and "bin/drydock" from executing under PHP 8.1, broadly because `null` is being passed to `strlen()`.
Test Plan: Ran `bin/phd debug task` and `bin/drydock ...` under PHP 8.1.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676, T13588
Differential Revision: https://secure.phabricator.com/D21795
Summary:
Ref T5479. Ref T13658. This was a contributed application from the early days of Phabricator which never had customers or users in the wild. The contributor moved on from the project many years ago.
Any capabilities in this general role would look different today. It also has one or two product name literal strings, so this is as good a time as any to remove it.
This change does not remove storage; I'll issue upgrade guidance and do that separately after some time.
Test Plan: Grepped for "phragment", got no relevant hits.
Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658, T5479
Differential Revision: https://secure.phabricator.com/D21793
Summary:
Ref T9530. Ref T13658. The "Releeph" application was never useful outside of Facebook and any application providing release support would not resemble it much.
It has some product name literal strings, so now is as good a time as any to get rid of it.
This application never left prototype and I'm not aware of any install in the wild that uses it (or has ever used it).
I did not destroy the database itself. I'll issue upgrade guidance and destroy the database in some future release, just in case.
Test Plan: Grepped for "releeph", found no relevant/removable hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658, T9530
Differential Revision: https://secure.phabricator.com/D21792
Summary: Ref T13658.
Test Plan: Static checks only, these are all reachable only by breaking your install in weird ways.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21781
Summary: Ref T13658.
Test Plan: Static checks only, these are mostly obscure or require breaking the install in unusual ways.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21780
Summary: Ref T13658.
Test Plan:
This is non-exhaustive.
- Started a JIRA auth provider creation workflow.
- Same for LDAP.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21779
Summary: Ref T13658.
Test Plan:
This is non-exhuastive.
- Poked around "Auth": edited a provider, toggled a provider.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21778
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Ran `bin/storage databases`.
- Viewed Badges UI exmaples page.
- Used eval rule for `strings.platform.server.name`, got "Phabricator".
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21773
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Viewed "remarkup.process" Conduit method API page.
- Viewed URIs in a Diffusion repository.
- Viewed editor protocol configuration in Settings.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21772
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Ran `bin/mail`.
- Uninstalled and reinstalled an application.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21770
Summary: Ref T13658
Test Plan:
This test plan is non-exhaustive.
- Looked at some of the config.
- Looked at guides.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21769
Summary: Ref T13658.
Test Plan:
This is non-exhaustive.
- Looked at some (most?) of the config values in Config.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21768
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Used Auth to start an "another copy of Phabricator" OAuth server workflow.
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21766
Summary: Ref T13658. I used the linter in D21763 to identify these and `split` them into arbitrary groups of 10 files.
Test Plan:
This test plan is non-exhaustive, because some of these strings are difficult to reach.
- Looked at "Create Service" in Almanac.
- Used "bin/auth" to go through a one-time auth workflow (not all related strings can be hit on a single workflow).
- Started the "Generate Keypair" worfklow in "SSH Public Keys".
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21765
Summary:
Ref T13667. The Herald "content added" rule (and other similar rules) do not correctly extract content from empty commits.
When we load an empty raw diff, return an empty changed content map.
Ref T13588. Also fix some PHP8.1 null/string stuff
Test Plan:
- Ran "bin/repository reparse --publish <commit>", with an empty commit hash and a nonempty commit hash.
- Reviewed Herald transcripts for general sanity.
Maniphest Tasks: T13667, T13588
Differential Revision: https://secure.phabricator.com/D21761
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
