1. 12 Jun, 2020 1 commit
  2. 07 Jun, 2020 1 commit
  3. 30 May, 2020 2 commits
  4. 28 May, 2020 2 commits
  5. 27 May, 2020 5 commits
    • epriestley's avatar
      Make lint tests handle paths better and distinguish between "0" and "null" more carefully · fce72b9c
      epriestley authored
      Summary:
      Ref T13543. Currently, the `cpplint` tests do not function because `cpplint` is passed a path which does not end in a suffix it recognizes.
      
      Change the tempfile / path code to pass `linter path/to/example.c`-style linters a path they expect.
      
      Then, correct some older code which was playing it fast-and-loose with "null" vs "0".
      
      Test Plan: Ran `arc unit --everything`, got a clean bill of health on all the linters I have installed. (This is probably not all tests, since I have only a subset of linters installed locally that we have code for.)
      
      Maniphest Tasks: T13543
      
      Differential Revision: https://secure.phabricator.com/D21291
      fce72b9c
    • epriestley's avatar
      (stable) Fix an issue when rendering a lint message which removes whitespace at the end of a file · 4aebaaf6
      epriestley authored
      Summary:
      Ref T13543. If a file ends in spaces and no newline, we'll emit a message suggesting removal of the spaces. This will effectively remove the line, but the code will then attempt to highlight text within the line.
      
      Prior to D21044 this continued without raising an error and produced a reasonable result, but it now fatals. Insetad, don't try to highlight lines which no longer exist.
      
      Test Plan: See T13543 for details.
      
      Maniphest Tasks: T13543
      
      Differential Revision: https://secure.phabricator.com/D21290
      4aebaaf6
    • epriestley's avatar
      Fix an issue when rendering a lint message which removes whitespace at the end of a file · e69aa326
      epriestley authored
      Summary:
      Ref T13543. If a file ends in spaces and no newline, we'll emit a message suggesting removal of the spaces. This will effectively remove the line, but the code will then attempt to highlight text within the line.
      
      Prior to D21044 this continued without raising an error and produced a reasonable result, but it now fatals. Insetad, don't try to highlight lines which no longer exist.
      
      Test Plan: See T13543 for details.
      
      Maniphest Tasks: T13543
      
      Differential Revision: https://secure.phabricator.com/D21290
      e69aa326
    • epriestley's avatar
      (stable) In the "cpplint" binding, raise messages on "line 0" without a line · 609c8c9f
      epriestley authored
      Summary:
      Ref T13543. The "cpplint.py" script may emit messages on line 0, but Arcanist doesn't accept these messages.
      
      This is a small piece of a whole set of broader issues, but stop the bleeding for now.
      
      Test Plan:
        - Ran `arc lint example.h` on a file with no `#ifndef` guard, and `cpplint` configured.
        - Cpplint raised a message at line "0".
        - Before change: arc choked when trying to render this.
        - After change: arc survives rendering.
      
      Maniphest Tasks: T13543
      
      Differential Revision: https://secure.phabricator.com/D21289
      609c8c9f
    • epriestley's avatar
      In the "cpplint" binding, raise messages on "line 0" without a line · 25ee39b6
      epriestley authored
      Summary:
      Ref T13543. The "cpplint.py" script may emit messages on line 0, but Arcanist doesn't accept these messages.
      
      This is a small piece of a whole set of broader issues, but stop the bleeding for now.
      
      Test Plan:
        - Ran `arc lint example.h` on a file with no `#ifndef` guard, and `cpplint` configured.
        - Cpplint raised a message at line "0".
        - Before change: arc choked when trying to render this.
        - After change: arc survives rendering.
      
      Maniphest Tasks: T13543
      
      Differential Revision: https://secure.phabricator.com/D21289
      25ee39b6
  6. 15 May, 2020 4 commits
    • epriestley's avatar
      (stable) Promote 2020 Week 19 · a5bfb968
      epriestley authored
      a5bfb968
    • epriestley's avatar
      (stable) Allow construction of a ConduitEngine with a bare ConduitClient · bee3bc05
      epriestley authored
      Summary:
      See PHI1735. "ConduitEngine" was once a future pool, but this has moved to "HardpointEngine". This class may no longer make much sense.
      
      In Phacility code, "bin/host upload" depends on using the Uploader, which needs a "ConduitEngine", not a "ConduitClient". This workflow may use asymmetric key signing, which "ConduitEngine" does not support.
      
      To unblock PHI1735, provide glue code between "Client" and "Engine". But a "more correct" change is probably removal of "Engine".
      
      Test Plan:
        - Ran `bin/host upload`, uploaded files (with additional changes to wrap the Client).
        - Created this revision.
      
      Differential Revision: https://secure.phabricator.com/D21260
      bee3bc05
    • epriestley's avatar
      Allow construction of a ConduitEngine with a bare ConduitClient · e3030ebc
      epriestley authored
      Summary:
      See PHI1735. "ConduitEngine" was once a future pool, but this has moved to "HardpointEngine". This class may no longer make much sense.
      
      In Phacility code, "bin/host upload" depends on using the Uploader, which needs a "ConduitEngine", not a "ConduitClient". This workflow may use asymmetric key signing, which "ConduitEngine" does not support.
      
      To unblock PHI1735, provide glue code between "Client" and "Engine". But a "more correct" change is probably removal of "Engine".
      
      Test Plan:
        - Ran `bin/host upload`, uploaded files (with additional changes to wrap the Client).
        - Created this revision.
      
      Differential Revision: https://secure.phabricator.com/D21260
      e3030ebc
    • Aviv Eyal's avatar
      update SSL error messge re:libphutil · 2d8156a7
      Aviv Eyal authored
      Test Plan: ╰(*°▽°*)╯
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21258
      2d8156a7
  7. 14 May, 2020 1 commit
    • epriestley's avatar
      Add "HTTPSFuture->addCurlOption()" for raw access to "curl_setopt()" · b76b9c40
      epriestley authored
      Summary: Fixes T13533. This is a narrow, fragile API for a particular Kerberos use case on one install.
      
      Test Plan:
      - Set a non-scalar key, got an exception.
      - Set <"duck", "quack">, got an exception from cURL that the value was invalid.
      - Set a bunch of made-up options to arbitrary values, no errors. cURL accepts anything so there's nothing we can do about this.
      - Set `CURLOPT_NOBODY` and saw the request behavior change, demonstrating that the call can produce effects.
      
      Maniphest Tasks: T13533
      
      Differential Revision: https://secure.phabricator.com/D21251
      b76b9c40
  8. 08 May, 2020 1 commit
  9. 04 May, 2020 2 commits
    • epriestley's avatar
      (stable) Fix an initialization issue in VectorTree · fb3e4014
      epriestley authored
      Summary: Ref T13520. In unusual cases where there are no changes in a changeset list (e.g., empty commits) we can fatal when trying to iterate over an empty list of vectors.
      
      Test Plan:
        - Created an empty commit.
        - Used "git show | pbcopy" to create a diff from it.
        - Viewed it in the web UI.
        - Before: fatal when iterating on `null`.
        - After: clean page.
      
      Maniphest Tasks: T13520
      
      Differential Revision: https://secure.phabricator.com/D21221
      fb3e4014
    • epriestley's avatar
      Fix an initialization issue in VectorTree · 6937d389
      epriestley authored
      Summary: Ref T13520. In unusual cases where there are no changes in a changeset list (e.g., empty commits) we can fatal when trying to iterate over an empty list of vectors.
      
      Test Plan:
        - Created an empty commit.
        - Used "git show | pbcopy" to create a diff from it.
        - Viewed it in the web UI.
        - Before: fatal when iterating on `null`.
        - After: clean page.
      
      Maniphest Tasks: T13520
      
      Differential Revision: https://secure.phabricator.com/D21221
      6937d389
  10. 01 May, 2020 5 commits
    • epriestley's avatar
      (stable) Promote 2020 Week 17 · 31c6b56b
      epriestley authored
      31c6b56b
    • epriestley's avatar
      Add "--browse" and "--input" to "arc paste", and remove "--json" (which had no effect) · af9faba0
      epriestley authored
      Summary:
      Ref T13528. For consistency with other commands ("arc upload", "arc diff"), support a "--browse" flag to "arc paste".
      
      Support "--input" as a more robust alternative to `x | y` (see T6996).
      
      Test Plan: Ran `arc paste --browse --input X`, got a new paste in a browser window. Ran other variations of flags and parameters.
      
      Maniphest Tasks: T13528
      
      Differential Revision: https://secure.phabricator.com/D21203
      af9faba0
    • epriestley's avatar
      Add "--browse" to "arc upload" and update behavior, particularly "--json" · c0d151e0
      epriestley authored
      Summary:
      Ref T13528. Provide a "--browse" flag to open files after they are uploaded.
      
      Update the code to use modern query strategies.
      
      This impacts the output of "--json", which was just raw "file.info" output before and could not represent the same path being passed several times (`arc upload X X`).
      
      Test Plan: Ran `arc upload` with `--browse` and `--json`.
      
      Maniphest Tasks: T13528
      
      Differential Revision: https://secure.phabricator.com/D21202
      c0d151e0
    • epriestley's avatar
      When recent PHP raises a "broken pipe" error in ExecFuture, treat it as a blocked stdin · 5448fe21
      epriestley authored
      Summary:
      Ref T13528. If we start a subprocess that immediately exits and then write to it, we can get a broken pipe error.
      
      Recent versions of PHP appear to raise this as an actual warning, and recent changes upgrade the warning to a runtime exception.
      
      I can't find any way to tell if the RuntimeException is a broken pipe or something else, except by examining the text of the error string.
      
      At least for now, treat this like a "blocked pipe" condition. Since the subprocess has exited and the bytes didn't write, this should generally be reasonable.
      
      Test Plan:
        - Viewed a file in Paste with an extension that Pygments does not have a lexer for.
        - This causes Pygments to exit immediately with an "unrecognized lexer" error. This closes the pipe, and the next write will fail with a broken pipe error.
        - Before patch: fatal on broken pipe.
        - After patch: clean resolution of the future and error condition.
      
      Maniphest Tasks: T13528
      
      Differential Revision: https://secure.phabricator.com/D21199
      5448fe21
    • epriestley's avatar
      When a proxy future wraps a future which throws an exception, resolve with an exception · a77cfb02
      epriestley authored
      Summary:
      Ref T13528. When you call `$future->resolve()`, we currently guarantee it is resolved by calling `FutureIterator->resolveAll()`.
      
      `resolveAll()` does not actually "resolve()" futures: it guarantees that they are ready to "resolve()", but does not actually call "resolve()".
      
      In particular, this means it does not throw exceptions.
      
      This can lead to a case where a Future has "resolve()" called directly (e.g., via a FutureProxy), uses "FutureIterator" to resolve itself, throws an exception inside "FutureIterator", the exception is captured and attached to the Futuer, then the outer future tries to access results. This fails since it's out-of-order.
      
      This can happen in practice with syntax highlighting futures, which may proxy pygments futures.
      
      Instead, "resolveAll()" before testing for exaceptions.
      
      Test Plan:
        - Locally, tried to highlight a Paste with an unrecognized lexer extension using Pygments.
        - Before patch: fatal when trying to access results of a Future with no results (because it has an exception instead).
        - After patch: resolution throws the held exception properly.
        - (See also next change.)
      
      Maniphest Tasks: T13528
      
      Differential Revision: https://secure.phabricator.com/D21198
      a77cfb02
  11. 30 Apr, 2020 4 commits
    • epriestley's avatar
      (stable) Work around "mb_check_encoding(<stringlike-object>)" warning in particular versions of PHP · c1d12ff7
      epriestley authored
      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
      c1d12ff7
    • epriestley's avatar
      Work around "mb_check_encoding(<stringlike-object>)" warning in particular versions of PHP · 696ec3f9
      epriestley authored
      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
      696ec3f9
    • epriestley's avatar
      (stable) Restore the ":(attr:filter=lfs)" test for LFS · ce7bd679
      epriestley authored
      Summary:
      See D21190. The ".gitattributes" approach fails when ".gitattributes" is in a subdirectory (or global). These are probably unusual cases, but at least one is known in the wild.
      
      Instead:
      
        - Restore the ":(attr:filter=lfs)" test, which seems to be the fastest accurate test available in modern Git.
        - If the test fails, assume the repository is not LFS. This only impacts users running very old versions of Git.
      
      Test Plan:
        - In LFS and non-LFS repositories, created diffs. Saw correct detection again.
        - Broke the command on purpose, saw LFS detection conclude "no LFS", but not fail disastrously.
      
      Subscribers: ptarjan
      
      Differential Revision: https://secure.phabricator.com/D21192
      ce7bd679
    • epriestley's avatar
      Restore the ":(attr:filter=lfs)" test for LFS · 284139a2
      epriestley authored
      Summary:
      See D21190. The ".gitattributes" approach fails when ".gitattributes" is in a subdirectory (or global). These are probably unusual cases, but at least one is known in the wild.
      
      Instead:
      
        - Restore the ":(attr:filter=lfs)" test, which seems to be the fastest accurate test available in modern Git.
        - If the test fails, assume the repository is not LFS. This only impacts users running very old versions of Git.
      
      Test Plan:
        - In LFS and non-LFS repositories, created diffs. Saw correct detection again.
        - Broke the command on purpose, saw LFS detection conclude "no LFS", but not fail disastrously.
      
      Subscribers: ptarjan
      
      Differential Revision: https://secure.phabricator.com/D21192
      284139a2
  12. 29 Apr, 2020 2 commits
  13. 28 Apr, 2020 1 commit
    • epriestley's avatar
      Replace "PhutilFileTree" with a more abstract "VectorTree" · 6ec09b2f
      epriestley authored
      Summary:
      Ref T13520. Replace "FileTree" with a "VectorTree" that does roughly the same thing. The major goals are:
      
        - Compress trees which contain sequences of child directories with no sibilings.
        - Build hierarchies of paths where path components may include renames.
      
      This is approximately similar to "FileTree" and similar to client logic in the new paths panel.
      
      Test Plan: See next change.
      
      Maniphest Tasks: T13520
      
      Differential Revision: https://secure.phabricator.com/D21182
      6ec09b2f
  14. 26 Apr, 2020 2 commits
  15. 25 Apr, 2020 2 commits
  16. 24 Apr, 2020 1 commit
  17. 15 Apr, 2020 1 commit
  18. 14 Apr, 2020 3 commits
    • epriestley's avatar
      If the Conduit server asserts it has the "gzip" capability, compress requests · 377ed2ed
      epriestley authored
      Summary:
      Ref T13507. For various messy reasons we can't blindly assume the server supports "gzip" -- but if the server tells us it does, we're on firmer ground.
      
      If the server returns an "X-Conduit-Capabilities: gzip" header and we have compression support locally, compress subsequent requests.
      
      This restores D21073, which was reverted by D21076.
      
      Test Plan: With a gzip-asserting server, added debugging code and ran various "arc" commands. Saw the 2nd..Nth calls hit compression code.
      
      Maniphest Tasks: T13507
      
      Differential Revision: https://secure.phabricator.com/D21119
      377ed2ed
    • epriestley's avatar
      If the Conduit client supports gzip, make calls with "Accept-Encoding: gzip" · a77da426
      epriestley authored
      Summary:
      Ref T13507. Add "Accept-Encoding: gzip" to requests if we can decompress responses.
      
      When we receive a compressed response, decompress it.
      
      Test Plan: Added debugging code, ran some commands, saw smaller payloads over the wire and inline decompression.
      
      Maniphest Tasks: T13507
      
      Differential Revision: https://secure.phabricator.com/D21118
      a77da426
    • epriestley's avatar
      In "phutil_loggable_string()", encode every byte above 0x7F · 890b57de
      epriestley authored
      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
      890b57de