1455127 - Remove dead features from upload.py

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Gregory Szorc [:gps]]

Attachment: Bug 1455127 - Remove functionality for uploading to a server;

With the transition to Taskcluster, "uploads" are artifacts in a local
directory. So we don't need to support uploading to a remote server
using SCP.

This commit removes all the code to support uploading to a remote
server.

And since property files were only written out for the remote case, all
that code can be deleted as well.

Since UPLOAD_HOST no longer means anything, we no longer set it in
mozharness configs.

Review commit: https://reviewboard.mozilla.org/r/237784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237784/

Comment 2

[Gregory Szorc [:gps]]

Attachment: Bug 1455127 - Remove --properties-file from upload.py;

This was only used for the remote server case. That code was recently
deleted.

Review commit: https://reviewboard.mozilla.org/r/237788/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237788/

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1455127 - Remove --package from upload.py;

The argument is never used in the local file copying function. So let's
nuke the argument.

Review commit: https://reviewboard.mozilla.org/r/237790/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237790/

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8969094 [details]
Bug 1455127 - Remove functionality for uploading to a server;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237784/diff/1-2/

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8969094 [details]
Bug 1455127 - Remove functionality for uploading to a server;

https://reviewboard.mozilla.org/r/237784/#review243562

::: build/upload.py:55
(Diff revision 2)
>      optional full path that is a base path of the local file, construct the
>      full remote path to place the file in. If base_path is not None, include
>      the relative path from base_path to file."""
>      if base_path is None or not local_file.startswith(base_path):
>          # Hack to work around OSX uploading the i386 SDK from i386/dist. Both
>          # the i386 SDK and x86-64 SDK end up in the same directory this way.

I bet you could remove this too!

::: build/upload.py:90
(Diff revision 2)
> -    key = OptionalEnvironmentVariable('UPLOAD_SSH_KEY')
> -    post_upload_command = OptionalEnvironmentVariable('POST_UPLOAD_CMD')
>  
>      if sys.platform == 'win32':
>          if path is not None:
>              path = FixupMsysPath(path)

This whole block is no longer necessary, since `path` will always be a local path.

::: build/upload.py:99
(Diff revision 2)
>                        action="store",
>                        help="Preserve file paths relative to this path when uploading. "
>                        "If unset, all files will be uploaded directly to UPLOAD_PATH.")
>      parser.add_option("--properties-file",
> -                      action="store",
> +                      action = "store",
>                        help="Path to the properties file to store the upload properties.")

It looks like `--properties-file` only got used in the remote case, so I think you should just remove this as well.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 8969098 [details]
Bug 1455127 - Remove --properties-file from upload.py;

https://reviewboard.mozilla.org/r/237788/#review243564

Oh, oops, I commented on this in the previous patch.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8969099 [details]
Bug 1455127 - Remove --package from upload.py;

https://reviewboard.mozilla.org/r/237790/#review243566

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8969094 [details]
Bug 1455127 - Remove functionality for uploading to a server;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237784/diff/2-3/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8969098 [details]
Bug 1455127 - Remove --properties-file from upload.py;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237788/diff/1-2/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8969099 [details]
Bug 1455127 - Remove --package from upload.py;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237790/diff/1-2/

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 8969094 [details]
Bug 1455127 - Remove functionality for uploading to a server;

https://reviewboard.mozilla.org/r/237784/#review243568

Comment 12

[(not currently active) Ted Mielczarek]

Comment on attachment 8969098 [details]
Bug 1455127 - Remove --properties-file from upload.py;

https://reviewboard.mozilla.org/r/237788/#review243570

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 8969099 [details]
Bug 1455127 - Remove --package from upload.py;

https://reviewboard.mozilla.org/r/237790/#review243572

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c8fe0facf52
Remove functionality for uploading to a server; r=ted
https://hg.mozilla.org/integration/autoland/rev/8527e727b92d
Remove --properties-file from upload.py; r=ted
https://hg.mozilla.org/integration/autoland/rev/ac17015816ca
Remove --package from upload.py; r=ted

Comment 15

[Andreea Pavel [:apavel]]

Backed out for messing with Windows 2012 build signing 

Push that started the failures (exceptions): https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ac17015816ca21649f102f50038a20a81b03babe

Failure log: https://taskcluster-artifacts.net/Z7lQV5LQTLmRAWLtTUNWOg/0/public/logs/chain_of_trust.log

Backout: https://hg.mozilla.org/integration/autoland/rev/46ea55337b33a0c5aa7b2cc71848139e7976f293

From IRC: 

If you look at the artifacts of the upstream task: https://tools.taskcluster.net/groups/fVXyeZxTT0OORpuTWoZ2_w/tasks/Ml6GI0sGSlKr-09tTWf3rg/runs/0/artifacts vs that of a previous commit: https://tools.taskcluster.net/groups/ZnKgGdqXTT29wYwA_eZztA/tasks/SV2AbNmeTgCo6BnJZKytWw/runs/0/artifacts notice that the path of setup.exe is different.

5:34 AM And the error from https://taskcluster-artifacts.net/Z7lQV5LQTLmRAWLtTUNWOg/0/public/logs/chain_of_trust.log is about not finding setup.exe.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64ec147eb854
Remove functionality for uploading to a server; r=ted
https://hg.mozilla.org/integration/autoland/rev/ac6e19238b99
Remove --properties-file from upload.py; r=ted
https://hg.mozilla.org/integration/autoland/rev/339c87677421
Remove --package from upload.py; r=ted

Comment 17

[Gregory Szorc [:gps]]

I restored some functionality to the first patch that likely caused the regression and relanded. It appears to have stuck this time.

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/64ec147eb854
https://hg.mozilla.org/mozilla-central/rev/ac6e19238b99
https://hg.mozilla.org/mozilla-central/rev/339c87677421