1. 01 Jun, 2021 1 commit
    • epriestley's avatar
      Correct an issue when winning "arc anoid" with certain terminal dimensions · 246e604a
      epriestley authored
      Summary: See PHI2085. Python 3 is stricter about integers and floats than Python 2 was, and we can end up passing a float where an integer was expected if the player wins "arc anoid" using a terminal with certain (most?) character dimensions.
      
      Test Plan:
        - Modified "arcanoid.py" to win instantly.
        - Adjusted terminal window to 80x24, ran "arc anoid", reproduced crash.
        - Ran "python2 arcanoid.py" and observed old victory animation behavior.
        - Applied patch.
        - Ran "arc anoid" and observed identical victory animation behavior.
      
      Differential Revision: https://secure.phabricator.com/D21667
      246e604a
  2. 30 May, 2021 1 commit
    • epriestley's avatar
      Avoid leaving stdin in nonblocking mode after a modern prompt · be1a4a91
      epriestley authored
      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
      be1a4a91
  3. 22 Mar, 2021 1 commit
    • epriestley's avatar
      On Windows, implement "Filesystem::copyFile()" with "copy()" · f0f95e5b
      epriestley authored
      Summary:
      Ref T13562. Currently, "Filesystem::copyFile()" uses "copy", which doesn't work now that we no longer invoke "cmd.exe" by default.
      
      Use "copy()" instead.
      
      Note that this whole function is probably nonsense, but I'll follow up on T13562.
      
      Test Plan:
        - Created a standalone script which runs "Filesystem::copyFile()".
          - Before: failed to copy any file.
          - After: succesfully copied normal files.
          - After: failed to copy a file over an existing directory with a reasonable error.
          - After: failed to copy a file over itself with a reasonable error.
      
      Maniphest Tasks: T13562
      
      Differential Revision: https://secure.phabricator.com/D21643
      f0f95e5b
  4. 16 Mar, 2021 1 commit
  5. 12 Mar, 2021 1 commit
  6. 11 Mar, 2021 1 commit
  7. 03 Mar, 2021 3 commits
    • epriestley's avatar
      In Arcanist, when trying to write to a file configuration source, create missing directories · 2d6452ac
      epriestley authored
      Summary: The ".git/arc" directory may need to be created when writing to working copy configuration.
      
      Test Plan:
        - Tried to store an answer to a prompt in a new working copy.
        - Before: error that ".git/arc" does not exist.
        - After: prompt saved to working copy configuration.
      
      Differential Revision: https://secure.phabricator.com/D21588
      2d6452ac
    • epriestley's avatar
      In "arc land", if rebasing a range fails, attempt to "reduce" it · 953d742a
      epriestley authored
      Summary:
      Ref T13576. See that task for discussion.
      
      When a user runs `arc land --pick A`, they may be selecting a range of commits ("X..Y") which have ancestors ("V..W") that should NOT land.
      
      We must slice "X..Y" out of history before we can merge it, to avoid landing changes from "V..W".
      
      When "X..Y" is simple and linear, we can rebase the range to pick our desired slice out of history.
      
      When "X..Y" includes merge commits, we frequently can not, and I could not identify any simple alternative. The best alternative I came up with is this "reduce" operation:
      
        - squash "into" onto Y, producing S, to guarantee there are no natural conflicts;
        - squash S onto X^, producing T, to get rid of the merge commits;
        - rebase T onto "into", producing R, to slice "X..Y" out of history;
        - squash R onto "into", producing Q. (R and Q will be the same, but this simplies the code.)
      
      This feels flimsy and fragile, but I can't immediately find a way to break it. See T13576 for more discussion.
      
      Test Plan:
        - Applied conflicting changes to `example.txt` in `master` and `feature1`.
        - Ran `arc land`, got a merge conflict.
        - Resolved the conflict with `git merge master`.
        - Ran `arc land`.
          - Before: merge conflict.
          - After: `arc land` resolves the merge correctly.
        - Stacked `feature2` on `feature1`, and made various mutations to `feature1` and `feature2`, then ran `arc land --pick feature2`. Changes made in `feature1` should not land, and they mostly do not. See T13576.
      
      Maniphest Tasks: T13576
      
      Differential Revision: https://secure.phabricator.com/D21590
      953d742a
    • epriestley's avatar
      Add a character marker to the "IMPLICIT COMMITS" warning in "arc land" · d72fad64
      epriestley authored
      Summary:
      Ref T13576. The "implicit commits" prompt in "arc land" shows a list of implicit and non-implicit commits.
      
      The implicit commits are marked with a background color, but this doesn't survive if you copy/paste the output into a support ticket.
      
      Make my life easier by also marking commits so the marker survives copy/paste.
      
      Test Plan: Ran "arc land" with implicit commits, saw a copy-pastable indicator.
      
      Maniphest Tasks: T13576
      
      Differential Revision: https://secure.phabricator.com/D21589
      d72fad64
  8. 01 Mar, 2021 1 commit
  9. 26 Feb, 2021 1 commit
    • epriestley's avatar
      Add a simple primitive for managing PHP runtime error logs · 6d60422d
      epriestley authored
      Summary:
      Ref T13624. If we want to send PHP errors to a log, using the "error_log" configuration option catches the broadest set of errors across versions of PHP.
      
      Configuring this disables errors on `stderr`, since they're sent to the log instead. We'd like them to go to both places; provide a simple wrapper for this. Also do a bit of writability testing.
      
      Test Plan: Wrote errors to a new log, see followup changes.
      
      Maniphest Tasks: T13624
      
      Differential Revision: https://secure.phabricator.com/D21578
      6d60422d
  10. 19 Feb, 2021 2 commits
  11. 10 Feb, 2021 1 commit
    • Vihang Mehta's avatar
      Update golint install instructions · f501f85e
      Vihang Mehta authored
      Summary:
      These are the instructions from https://github.com/golang/lint.
      
      The fetch location changed to `golang.org/x/lint/golint` from `github.com/golang/lint/golint`.
      `-u` tells go to update the package and its deps if they exist.
      `-u` Shouldn't strictly be necessary, but figured we might as well follow the instructions from `golint`.
      
      Test Plan:
      Enable golint without having it installed.
      Ensure that the install instructions now show the new location.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21552
      f501f85e
  12. 08 Feb, 2021 1 commit
  13. 03 Feb, 2021 5 commits
  14. 13 Jan, 2021 1 commit
  15. 11 Jan, 2021 8 commits
    • Jessica Clarke's avatar
      Fix pyflakes tests for recent pyflakes versions · 17238126
      Jessica Clarke authored
      Summary:
      Since 2.1.0 (commit 75bc0c03c145), pyflakes has included the Python
      version and platform in its version output, so ignore it if present.
      
      Since 2.2.0 (commit 6ba3f8e0b59b), pyflakes has included the column
      number in its messages, so update the parser to include it and drop the
      column number from the (only) test in order to work with both old and
      new versions. Whilst here, assign names to the capture groups to make
      the code clearer.
      
      Test Plan: Ran arc unit
      
      Reviewers: epriestley, joshuaspence, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21504
      17238126
    • Jessica Clarke's avatar
      Fix ArcanistJSHintLinterTestCase::testLinter for recent JSHint · 09cff861
      Jessica Clarke authored
      Summary:
      Recent JSHint improves the warning and attributes it to the equals sign
      rather than the end of the expression (changed in 897e0359ce19, first
      released in 2.11.0-rc1).
      
      Test Plan: Ran arc unit with JSHint 2.12.0
      
      Reviewers: epriestley, joshuaspence, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21503
      09cff861
    • Jessica Clarke's avatar
      Fix PhutilOAuth1FutureTestCase::testOAuth1SigningWithJIRAExamples for PHP 8 · f64eb043
      Jessica Clarke authored
      Summary:
      PHP 8 deprecates openssl_free_key as the key is automatically freed, so
      silence the warning in PhutilOAuth1Future::signString.
      
      Test Plan: Ran arc lint --everything
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T13588
      
      Differential Revision: https://secure.phabricator.com/D21502
      f64eb043
    • Jessica Clarke's avatar
      Fix PhutilUTF8TestCase::testUTF8Convert for PHP 8 · 9589fd18
      Jessica Clarke authored
      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
      9589fd18
    • Jessica Clarke's avatar
      Fix ArcanistFormattedStringXHPASTLinterRule on older PHP after D21500 · 687cb41a
      Jessica Clarke authored
      Summary:
      Calling 'Foo::bar' is only supported since PHP 7, whereas the array form
      is supported since PHP 5.4, which is below our PHP 5.5 baseline.
      
      Test Plan: No regressions under PHP 8 and snippet tested on 3v4l
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21505
      687cb41a
    • Jessica Clarke's avatar
      Fix ArcanistFormattedStringXHPASTLinterRule for PHP 8 · 90ac9a2f
      Jessica Clarke authored
      Summary:
      PHP 8's sprintf raises a ValueError when encountering unknown format
      specifiers (previously it would eat the argument and print nothing), so
      linting format strings like %Ls dies with an uncaught ValueError.
      
      Fix this by using a custom callback during linting to turn all format
      specifiers into %s and replace the dummy null argument with the original
      format specifier, ensuring we always end up providing valid input to the
      sprintf at the end. This has the nice property that the output of the
      call to xsprintf is the original format string, though any
      transformation into valid input would do.
      
      Test Plan: Ran arc lint
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T13588
      
      Differential Revision: https://secure.phabricator.com/D21500
      90ac9a2f
    • Jessica Clarke's avatar
      Fix PhutilTypeSpec's regex handling for PHP 8 · 0adef03f
      Jessica Clarke authored
      Summary:
      In previous versions, passing the wrong type to preg_match would give a
      warning that could be suppressed by @ and caught by set_error_handler,
      but as of PHP 8 this raises a TypeError and so remains uncaught. Thus
      check up-front whether the provided value is a string.
      
      This fixes linting arc itself when run with PHP 8, as includes and
      excludes use "optional regex | list<regex>", so would previously try to
      pass an array to preg_match for the first alternative and die.
      
      Test Plan: Ran arc lint
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T13588
      
      Differential Revision: https://secure.phabricator.com/D21499
      0adef03f
    • Jessica Clarke's avatar
      Fix error handler on PHP 8 · 446dcf1c
      Jessica Clarke authored
      Summary:
      PHP 7.2.0 deprecated the 5th parameter and PHP 8 removed it, so stop
      using it and provide a default value to avoid erroring with:
      
      ```
      Too few arguments to function PhutilErrorHandler::handleError(), 4 passed and exactly 5 expected
      ```
      
      Test Plan: Used to create this revision with PHP 8 on macOS
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T13588
      
      Differential Revision: https://secure.phabricator.com/D21498
      446dcf1c
  16. 10 Jan, 2021 2 commits
    • Jessica Clarke's avatar
      Suppress PHP 8 deprecation warning in __arcanist_init_script__ · 930f7e11
      Jessica Clarke authored
      Summary:
      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: 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/D21497
      930f7e11
    • Jessica Clarke's avatar
      Remove final from private functions for PHP 8 compatibility · 3ab2b407
      Jessica Clarke authored
      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
      3ab2b407
  17. 16 Oct, 2020 2 commits
  18. 30 Sep, 2020 2 commits
    • bootstraponline's avatar
      Fix rubocop lint tests · 04e340ab
      bootstraponline authored
      Summary: Fix tests to work with rubocop 0.92.0 released on September 25, 2020
      
      Test Plan: Unit tests pass
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: Korvin, epriestley
      
      Differential Revision: https://secure.phabricator.com/D21474
      04e340ab
    • epriestley's avatar
      Fix "PhutilOpaqueEnvelopeTestCase" under PHP 7.4 with "zend.exception_ignore_args" · 524aa2ae
      epriestley authored
      Summary:
      See PHI1894. PHP 7.4 introduced a new runtime configuration option, "zend.exception_ignore_args", which removes the "args" from exception backtraces.
      
      The "PhutilOpaqueEnvelopeTestCase" relies on this behavior (since it explicitly inspects stack frames). Although the test isn't critical and could be restructured, it seems like there is little value to ever enabling this option in the context of Phabricator.
      
      Disable it at startup so environments are more consistent across different PHP versions and configurations.
      
      Test Plan:
        - Enabled "zend.exception_ignore_args" under PHP 7.4.
        - Ran "PhutilOpaqueEnvelopeTestCase".
        - Before: failure, expected signpost value not present in stack trace (because no "args" are present on the exception).
        - After: test passes.
      
      Differential Revision: https://secure.phabricator.com/D21473
      524aa2ae
  19. 28 Sep, 2020 1 commit
    • Paul Tarjan's avatar
      fail `arc diff` if second lfs push errors · 7597f31b
      Paul Tarjan authored
      Summary:
      We are having issues where people run out of file descriptors and the first `git push` will succeed, but the
      second one will not. We'd like the diff to not be created in this case as it leads to weird behavior like our tests
      running against 0 changed files.
      
      Test Plan: none
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D21471
      7597f31b
  20. 18 Sep, 2020 1 commit
  21. 17 Sep, 2020 2 commits
  22. 16 Sep, 2020 1 commit