Summary:
Note that booleans are scalars. Full stop.
| is_scalar($v) | Result |
|------------------|--------|
| `"foo"` | true |
| `""` | true |
| `null` | true |
| `0` | true |
| `0.5` | true |
| `true` | true |
| `false` | true |
| `new stdclass()` | false |
| `array()` | false |
Note that phutil_nonempty_scalar() was designed just to tell
whenever a scalar is "empty" or not. So it must not explode
when receiving a valid scalar.
So the question is not whenever a boolean is a scalar or not,
but whenever is empty or not. But also this is a clear fact:
| `$v` | `is_scalar($v)` | `!is_empty($v)` | `if(strlen($v))`|
|---------|-----------------|-----------------|-----------------|
| `true` | `true` | `true` | `true` |
| `false` | `true` | `false` | `false` |
In short, now the function does not explode anymore with bool
values. Instead, it says whenever are empty or not.
In bold the exact changes:
| Value |`phutil_nonempty_scalar($v)`|
|-------------------|----------------------------|
| `"foo"` | true |
| `""` | false |
| `null` | false |
| `0` | true |
| `0.5` | true |
|`obj` with tostring| true |
|`obj` withno tostr.| Exception |
| `array()` | Exception |
| `true` | ~~Exception~~ **true** |
| `false` | ~~Exception~~ **false** |
Closes T15239
Test Plan:
- check if it makes sense to you
- check the few usages
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15239
Differential Revision: https://we.phorge.it/D25117
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_stringlike()` as a replacement for string-alike variables.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_stringlike() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
Closes first half of T15331
Test Plan: Applied these two changes (one in Arcanist, one in Phorge). Project with empty Description field was created and `/project/view/projectid/` rendered in web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan, speck
Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15331
Differential Revision: https://we.phorge.it/D25176
Summary:
`array_fuse` in Arcanist is a wrapper for calling `array_combine($list, $list)`.
The latter doesn't accept passing `null` in PHP 8.2.
Going to `/conduit/method/project.create/`, entering a `name` but nothing as `members` (so we pass `null`), and calling this method, an exception is thrown.
Thus make `array_fuse` accept null and return an empty list in such cases.
Closes T15393
Test Plan: Applied this change; afterwards "Method Call Result" page at `/api/project.create` correctly displayed in the web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15393
Differential Revision: https://we.phorge.it/D25228
Summary:
After PHP 8.1 the function `rawurlencode()` does not accept anymore the `null` value.
Thus return an empty string when the input parameter is null instead of passing the input parameter to `rawurlencode()`.
Closes T15263
Test Plan:
Applied this change on top of D25144, D25145, D25146, D25147, D25151,
D25152, D25153 and D25163 and already existing Workboard located at
`/project/view/1/` finally rendered in web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15263
Differential Revision: https://we.phorge.it/D25164
Summary:
Passing `null` to the `$string` parameter of `mb_convert_case()` is deprecated in PHP 8.1.
This is one of the exceptions which block rendering the "Browse Projects" overlay dialog.
Closes part of T15335
Test Plan: Applied this change in Arcanist (plus the four changes in D25179 in Phorge) and the `Browse Projects` overlay dialog finally rendered in web browser and listed existing projects.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15335
Differential Revision: https://we.phorge.it/D25180
Summary:
Ref T13675. When a process daemonizes, it needs to close these actual file descriptors, so the calling context must provide them.
Elsewhere, modify the embedded "arc" to provide them. Then use them in place of reopening the streams.
Test Plan: Ran locally with embedded "arc", will deploy for production hangs.
Maniphest Tasks: T13675
Differential Revision: https://secure.phabricator.com/D21804
Summary:
Ref T13675. Ref T13556. The "STDOUT" and "STDERR" constants are defined by the PHP CLI SAPI, in `cli_register_file_handles()`.
The "native arc" embedded PHP wrapper doesn't define these, and there's no real reason to define them, since they're just defined in terms of the PHP stream wrappers ("php://stdin", etc) anyway.
This patch isn't exhaustive (and a subsequent change should add lint, rejecting these magic constants) but is just trying to make native `arc` functional.
Test Plan: Created this revision with a standalone native `arc` binary.
Subscribers: cspeckmim
Maniphest Tasks: T13675, T13556
Differential Revision: https://secure.phabricator.com/D21794
Summary:
Ref T13658. Remove all product name literals from "pht()" strings, by replacing them with generic text where that feels reasonably natural, or "PlatformSymbols" calls elsewhere.
These calls were identified with `arc lint --everything` after enabling the lint rule in D21763.
Test Plan: Read strings, ran "arc".
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21764
Summary: Ref T13588. Adds "phutil_nonempty_string()" and similar methods to support PHP8.1 compatibility.
Test Plan: Added unit tests, ran unit tests.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21762
Summary: Ref T13588. I pointed my local `php` at PHP 8.1 and this is what I've hit so far; all these cases seem very unlikely to have any subtle behavior.
Test Plan: Ran various `arc` workflows under PHP 8.1.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21742
Summary:
Ref T13649. Currently, "arc" may leave stdin nonblocking after showing a prompt. This can cause various odd behaviors down the line.
I can't immediately reproduce this behavior on macOS in "zsh" or "bash" (I'm unable to get stdin to remain nonblocking beyond the process lifespan), and also don't have pcntl locally so there's a fair amount of handwaving here.
Test Plan: This is somewhat speculative since I can't immediately reproduce the behavior. I tested the locally-reachable paths (no pcntl) but they're not interesting.
Maniphest Tasks: T13649
Differential Revision: https://secure.phabricator.com/D21666
Summary: Ref T13608. Ref T13100. Ref T13586. Properly checking "preg_match()" and similar calls for failure and raising useful exceptions is complicated and error-prone. Provide wrapper functions with an API that's more consistent with the rest of the codebase: matches are returned; and errors raise detailed exceptions.
Test Plan: See next change.
Maniphest Tasks: T13608, T13586, T13100
Differential Revision: https://secure.phabricator.com/D21561
Summary:
In PHP 8 passing an invalid encoding to mb_convert_encoding raises a
ValueError (which extends Error not Exception), so fix the test to also
catch Throwable (but leave the explicit Exception case for PHP 5, which
lacks Throwable).
Test Plan: Ran arc unit --everything
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21501
Summary:
This combination does not make sense and PHP 8 errors with:
```
Private methods cannot be final as they are never overridden by other classes
```
Thus remove the redundant final from all such functions.
Test Plan: Used to create this revision with PHP 8 on macOS
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D21496
Summary:
See <https://discourse.phabricator-community.org/t/search-by-name-in-files-doesnt-support-number/4300>.
I can't exactly reproduce the original issue, but when a query like "quack 1234" is tokenized, we end up calling "phutil_utf8v(1234)", where the argument is an integer.
At least in recent versions of PHP, this fatals ("trying to access an offset of an integer"). Cast the argument first.
Test Plan: Searched for "quack 1234" in Files. Before: fatal accessing offset of integer; after: correct results.
Differential Revision: https://secure.phabricator.com/D21477
Summary:
Ref T13546. Pull some small utility changes out of the deeper stack of "land/markers" changes.
"phutil_partition()" makes it easier to write code that loops over a list grouping elements, then acts on each group. This kind of code is not terribly common, but often feels awkward when implemented with raw primitives.
"msortv()" can support "natural" sorting, which sorts "feature1", "feature2", ..., "feature10" in a more human-readable order.
Test Plan: Ran unit tests, used new behaviors elsewhere in "arc markers" workflows.
Maniphest Tasks: T13546
Differential Revision: https://secure.phabricator.com/D21371
Summary: This simplifies merging a list-of-lists, which is a common pattern when asking a list of extensions to each provide some kind of list of items.
Test Plan: Used elsewhere in Piledriver.
Differential Revision: https://secure.phabricator.com/D21294
Summary:
With some frequency, code wants to assert that some "$o->m()" returns a list of objects of type X, possibly with unique values for some "getKey()"-style method result.
Existing checks via `PhutilTypeMap` and `assert_instances_of()` aren't quite powerful enough to do this while producing an easily understandable error state. We want to know that the error arose from a call to "$o->m()" in particular.
Test Plan: Currently used elsewhere, in Piledriver code.
Differential Revision: https://secure.phabricator.com/D21293
Summary: Fixes T13527. Some versions of PHP strictly require that we pass a string value, and reject "stringlike" objects (objects which implement "__toString()").
Test Plan: Ran unit test, although this is somewhat aspirational because my local PHP version isn't affected.
Maniphest Tasks: T13527
Differential Revision: https://secure.phabricator.com/D21193
Summary:
Ref T13507. Currently, this function is a bit conservative about what it encodes, and passing it a string of binary garbage may result in an output which is not valid UTF8.
This could be refined somewhat, since it's less than ideal if the input has valid UTF8. The ideal behavior for byte sequences where all bytes are larger than 0x7F is probably a variation of "phutil_utf8ize()" that replaces bytes with "<0xXX>" instead of the Unicode error glyph.
For now, just err on the side of mangling.
Test Plan: Dumped various binary payloads in the new gzip setup check, saw sensible output in the web UI.
Maniphest Tasks: T13507
Differential Revision: https://secure.phabricator.com/D21117
Summary: Ref T13490. Make terminal strings work more like HTML does in Phabricator, and make it easier to display refs.
Test Plan: Added some display code, ran `arc inspect` to hit it, saw a nice ref printed.
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21093
Summary: Ref T11968. Phage has another "sustained pool of Futures" use case, and needs some slight adjustments after Future API changes.
Test Plan: Ran `bin/phage status ...`, got a clean result instead of a JSON decoding failure.
Maniphest Tasks: T11968
Differential Revision: https://secure.phabricator.com/D21058
Summary:
Depends on D21001. Ref T13490.
- Require "--" to terminate arguments when running noninteractively.
- Don't correct spelling when running noninteractively (we still suggest corrections).
- Remove old "phage" wrapper script.
- Fix an issue where "argv" could implicitly generate a parseable "--argv" flag.
- Accept "--" as an argument terminator even when there is no argument list.
- Update some strings to use more modern quoting style.
- Make workflows decline to handle signals by default.
- Allow "arc weld" to weld non-UTF8 files together.
Test Plan: Ran various commands. Welded non-UTF8 files.
Maniphest Tasks: T13490
Differential Revision: https://secure.phabricator.com/D21002
Summary:
Ref T13395. Merge a lot of stuff which doesn't break existing workflows:
- Merge a UTF8 fix for "cmd.exe" on Windows.
- Merge minor changes to JSON linters.
- Merge some shell completion stuff.
- Merge some "arc anoid" fixes.
- Merge various Windows improvements to unit tests which interact with processes / the filesystem.
- Merge some other Windows path fixes.
- Merge a UTF8 character class fix.
- Merge script initialization.
- Merge unit test support scripts.
- Merge some initialization code.
- Merge Windows stdout/stderr-as-files code.
- Merge a bunch of code for making exec tests work on Windows.
- Merge more Windows unit test fixes.
- Merge "continue on failure" mode when loading symbols.
- Merge "GPC" order CLI fixes.
Test Plan: Ran `arc unit --everything`; created this change. There's likely some less-than-perfect code here.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20988
Summary: Ref T13395. Moves all remaining code in "libphutil/" into "arcanist/".
Test Plan: Ran various arc workflows, although this probably has some remaining rough edges.
Maniphest Tasks: T13395
Differential Revision: https://secure.phabricator.com/D20980
