1370694 - vcssync code refactoring

Status: RESOLVED FIXED

(Developer Services :: Servo VCS Sync, enhancement)

Description

[Gregory Szorc [:gps]]

Creating bug for misc patches I wrote in pursuit of bug 1357597.

Comment 1

[Gregory Szorc [:gps]]

Attachment: vcssync: install nose in virtualenv (bug 1370694);

./run-tests uses nose to run Python-based unit tests. So install it so
we can start writing them.

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

Comment 2

[Gregory Szorc [:gps]]

Attachment: vcssync: framework for test only CLI commands (bug 1370694);

We will introduce some CLI commands that are only really useful for
testing. Add some logic to to conditionally include commands
depending on the presence of an environment variable.

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

Comment 3

[Gregory Szorc [:gps]]

Attachment: testing: check for pip when determining if virtualenv exists (bug 1370694);

Previously, we'd look for a virtualenv directory when determining if
we needed to create a virtualenv. I'm not sure how, but periodically
I'll have an empty virtualenv directory and `create-environment` will
fail. So, change the check to look for the pip executable instead.

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

Comment 4

[Gregory Szorc [:gps]]

Attachment: vcssync: upgrade to Mercurial 4.2 (bug 1370694);

All the tests pass with this version. As a bonus, the development
environment now runs 4.2, so any local Mercurial configs or
extensions tailored for 4.2 (such as built-in support for pager
and color) just work. (This was actually what spurred me to do this:
I didn't like my virtualenv's hg 4.1 not working like 4.2.)

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

Comment 5

[Gregory Szorc [:gps]]

Attachment: vcssync: support deleting refs in `git update-ref` helper (bug 1370694);

We'll need this in a subsequent commit. As part of this, I also
discovered that we weren't emitting a trailing null byte in the command
string. The Git man pages imply this is the proper format and sure
enough I did manage to get `git update-ref` to complain with it
missing and work with it present.

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

Comment 6

[:glob ✱]

Comment on attachment 8875025 [details]
vcssync: install nose in virtualenv (bug 1370694);

https://reviewboard.mozilla.org/r/146374/#review152764

::: vcssync/test-requirements.txt:14
(Diff revision 1)
> +nose==1.3.7 \
> +    --hash=sha256:dadcddc0aefbf99eea214e0f1232b94f2fa9bd98fa8353711dacb112bfcbbb2a

may as well bump the version of nose in other /test-requirements.txt and testing/requirements-hgdev.txt from 1.3.4 to 1.3.7

Comment 7

[:glob ✱]

Comment on attachment 8875026 [details]
vcssync: framework for test only CLI commands (bug 1370694);

https://reviewboard.mozilla.org/r/146376/#review152770

::: vcssync/setup.py:14
(Diff revision 1)
> +if 'VCSSYNC_ENABLE_TESTING_COMMANDS' in os.environ:
> +    console_scripts.extend([
> +    ])

this code doesn't do anything.  better to add this when it's required rather than have dead code in the repo.

Comment 8

[:glob ✱]

Comment on attachment 8875027 [details]
testing: check for pip when determining if virtualenv exists (bug 1370694);

https://reviewboard.mozilla.org/r/146378/#review152774

Comment 9

[:glob ✱]

Comment on attachment 8875028 [details]
vcssync: upgrade to Mercurial 4.2 (bug 1370694);

https://reviewboard.mozilla.org/r/146380/#review152776

::: commit-message-3280e:6
(Diff revision 1)
> +and color) just work. (This was actually what spurred me to do this:
> +I didn't like my virtualenv's hg 4.1 not working like 4.2.)

heh; a long time ago i added a workaround to my dev env so command line hg actions use my system-installed mercurial instead of the venv's.

Comment 10

[:glob ✱]

Comment on attachment 8875029 [details]
vcssync: support deleting refs in `git update-ref` helper (bug 1370694);

https://reviewboard.mozilla.org/r/146382/#review152778

like some of the other changes here i'm not convinced this is actually refactoring, and it seems out of place here, but it looks fine.

Comment 11

[:glob ✱]

Comment on attachment 8875026 [details]
vcssync: framework for test only CLI commands (bug 1370694);

https://reviewboard.mozilla.org/r/146376/#review154252

sorry, let's land this as-is.  i'll race you to get something landed that uses it.

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8875026 [details]
vcssync: framework for test only CLI commands (bug 1370694);

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

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8875027 [details]
testing: check for pip when determining if virtualenv exists (bug 1370694);

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

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8875029 [details]
vcssync: support deleting refs in `git update-ref` helper (bug 1370694);

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b97c7c9cd436
vcssync: framework for test only CLI commands ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/7676a8af6b2d
testing: check for pip when determining if virtualenv exists ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/e4bcd39b49ee
vcssync: support deleting refs in `git update-ref` helper ; r=glob