1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-12 18:02:39 +01:00
Commit graph

12 commits

Author SHA1 Message Date
epriestley
18799c1829 Switch file uploader in "arc diff" to use ConduitEngine
Summary: Fixes T13498. Currently, `arc diff` may invoke the file upload flow using an older variation of the API. Both the modern and fallback clients build an Engine, so switching to the modern API is trivial.

Test Plan:
  - Created a local commit with a 1MB binary file.
  - Ran `arc diff --only`.
  - Before patch: fatal on old API call.
  - After patch: clean upload.

Maniphest Tasks: T13498

Differential Revision: https://secure.phabricator.com/D21043
2020-03-20 12:19:17 -07:00
epriestley
1b97f8b408 Update "arc upload" for Toolsets
Summary: Ref T13490. This largely makes "arc upload" work, although the Future stuff is still a bit wonky. See T11968.

Test Plan: Ran "arc upload README.md", got an upload.

Maniphest Tasks: T13490

Differential Revision: https://secure.phabricator.com/D21030
2020-02-26 08:22:18 -08:00
epriestley
a59cfca5f1 Upgrade "arc upload" to use SHA256
Summary: Fixes T12464. Moves "arc upload" to SHA256 where applicable.

Test Plan: Ran `arc upload` against a server with D17620 twice, saw it skip the actual upload the second time.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12464

Differential Revision: https://secure.phabricator.com/D17622
2017-04-04 16:56:35 -07:00
epriestley
c4e84550fc Allow "arc upload" to work correctly if it can not hash content
Summary:
Ref T12464. This is similar to D17619 and prepares us to move to SHA256 in the client.

Note that it's fine if `arc` and Phabricator disagree about hashing algorithms. We don't really trust the client anyway, so if things are mismatched clients will just end up transferring a bit more data instead of getting to cheat when Phabricator already has copies of data.

Test Plan: Ran `arc upload`, got a clean upload.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12464

Differential Revision: https://secure.phabricator.com/D17621
2017-04-04 16:24:27 -07:00
epriestley
6507be27ae Fix handling of View Policy in CLI upload workflow for small, unique files
Summary:
Ref T7148. When uploading files from the CLI with a particular view policy, we may not respect it if the file is unique (so the data isn't already known) and small (so it doesn't invoke the chunker).

This is rare (and may never have happened outside of testing) because:

  - production dumps are always larger than the minimum chunk size;
  - only cluster stuff uses `setViewPolicy()`;
  - the default policy is "Administrators" anyway, which is safe.

However, I caught it in local testing, so fix it up.

Test Plan: Used `bin/host upload --file ...` to upload a small, unique file. Verified it uploaded with the correct custom view policy ("No One") rather than the default view policy ("Administrators").

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7148

Differential Revision: https://secure.phabricator.com/D16408
2016-08-17 09:04:01 -07:00
epriestley
4a1160e0c3 When pushing changes to staging, also push the base commit
Summary:
Fixes T10509. Pushing changes to staging can be inefficient. What happens, roughly, is:

  - Master is at commit "W" -- "W" is the most recent published commit in the main repository.
  - The local working copy has one change on top of that, "X", so its history is commits "A, B, C, D, E, F, ..., U, V, W, X".
  - The remote has some other previous changes that I or other users have made, maybe like "A, B, C, ..., S, T, U, Y" and "A, B, C, ..., T, U, V, Z", from previous pushes to staging areas.
  - "X", "Y" and "Z" will never actually make it to master, because they'll be squash-merged/amended by `arc land`.

So the local says "I want to push 'X'", and the remote says "I know about 'Y' and 'Z', are those in the history of 'W'? You only need to send me new stuff if they are".

But they aren't, so the local says "nope, so here's the whole history for you". This is slow and sends a ton of data that the remote already has over the network.

In theory, Git could use a slightly different algorithm to tell the local about more commits, but this is hard, rarely useful, and not the kind of thing I'd be excited about changing if I was the Git upstream.

Instead, when pushing "X", also push "W", to trick Git into telling future clients about it.

Now, the remote should say "I know about 'W', 'Y' and 'Z'", and the local will say "oh, great, 'W' is in history, here's just the changes since then".

Also, fail `arc diff` if the push to staging fails, and tell users to use `--skip-staging`. This code has been in production for a while and doesn't seem to have any issues, and a failed push to staging prevents builds, lands, etc.

Test Plan:
  - Ran `arc diff`, saw two changes push.
  - Ran `arc diff --base arc:empty`, saw only one change push.
  - Ran `arc diff` with an intentionally broken staging area, saw an error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10509

Differential Revision: https://secure.phabricator.com/D15424
2016-03-07 06:53:49 -08:00
Joshua Spence
2db40f9953 Various translation improvements
Summary: Depends on D14070.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14072
2015-11-02 21:31:04 +11:00
epriestley
55d9cc7013 Improve temporary file upload API and add viewPolicy support
Summary: Ref T7148. In D14056, I let `arc upload` upload temporary files, but this is a better way to do some of the internals. Also add support for setting a `viewPolicy`.

Test Plan: Used `arc upload`, `arc upload --temporary`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7148

Differential Revision: https://secure.phabricator.com/D14075
2015-09-07 12:44:59 -07:00
epriestley
9896908685 Add temporary file support to ArcanistFileUploader
Summary: Ref T7148. Depends on D14055. Allows files to be marked as temporary when uploaded.

Test Plan: Used `arc upload --temporary` to upload temporary files.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7148

Differential Revision: https://secure.phabricator.com/D14056
2015-09-04 10:34:14 -07:00
epriestley
a36dc81715 Provide more documentation for Arcanist upload stuff
Summary: Ref T8259. Make some highly-ambiguous methods like `setData()` more clear.

Test Plan: reading

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13038
2015-05-27 10:26:27 -07:00
epriestley
5c7b22e620 Use file.allocate to upload large files from arc diff
Summary: Fixes T8259. Depends on D13016. Use modern logic to support large file uploads.

Test Plan:
  - Diffed with some images, saw them show up in the diff.
  - Diffed with a 12MB binary, saw it upload in chunks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13017
2015-05-27 10:26:12 -07:00
epriestley
877e7b6388 Move Conduit file upload logic into a separate class
Summary:
Ref T8259. Currently, `arc upload` uses new logic but `arc diff` uses older logic internally. This prevents `arc diff` from uploading files larger than 4MB to newer servers.

Split the upload logic apart so the two upload pathways can share it. Callers now build a list of FileDataRefs and hand them to an Uploader for upload.

Test Plan:
Ran `arc upload` on:

  - One file.
  - Several files.
  - Small files.
  - Big files.
  - Directories.
  - Unreadable files.
  - Files full of random data.
  - The same file over and over again.
  - The same big file over and over again.
  - Artificially broke `file.allocate` and redid some of the simple cases (large/chunked aren't expected to work in this case).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13016
2015-05-27 10:25:53 -07:00