1471920 - [mozversion] Add support for python 3

Status: RESOLVED FIXED

(Testing :: Mozbase, enhancement, P3)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

+++ This bug was initially created as a clone of Bug #1427568 +++

We need to add python 3 support to mozversion without losing legacy python support. Whilst some aspects were taken care of in bug 1427568 we are now able to run the unit tests against Python 3, and many of these are failing.

To work on this bug you will need to install and configure Mercurial, which will enable you to download the Firefox source code. It will also be used to commit your changes locally and prepare a patch for review. See the installation guide at https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/installing.html for help getting Mercurial on your system. Once installed, there are some extensions we recommend installing, details of these can be found here: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/extensions.html

To clone the Firefox source code we recommend using the unified repository. Details of this and how to create a bookmark for your work can be found here: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/unifiedrepo.html

As this bug relates to Python 3, you will need to have this installed on your system. Our continuous integration is currently using Python 3.5, so for best results we recommend using the same version locally. There are a number of ways to install Python, and they vary depending on your environment. We suggest reading over the guides at http://docs.python-guide.org/en/latest/starting/installation/#python-3-installation-guides to find the best method for you. Note that we also need to maintain support for Python 2, so you'll also need to have Python 2.7 installed.

Whilst we're moving towards adding support for Python 3, we've disabled any tests that fail against this version. This means that in order to work on this bug you will need to enable the tests. This can be done by removing "skip-if = python == 3" from the manifest file in `testing/mozbase/mozversion/tests/manifest.ini`

To run the tests against Python 3, execute the following command:

```
mach python-test --python=3.5 testing/mozbase/mozversion
```

Work through the test failures, and update the tests and source code for the package to simultaneously support Python 2.7 and Python 3.5. You can always check that your changes have not inadvertently broken support for Python 2 by using `--python=2.7` on the command line. 

If you're new to Python 3, the guide at https://docs.python.org/3/howto/pyporting.html may help you to get started with understanding the differences. To assist with supporting both Python 2 and 3, we have vendored the "six" package. You can read more about this and how to use it at https://pythonhosted.org/six/

Note that this package depends on others that may not yet support Python 3. If these dependencies are preventing tests from passing, then we can block this bug on getting support in those packages. If there are circular dependencies then we may need to coordinate landing a sequence of patches between bugs.

Once the package supports Python 3 and the tests pass, we will also need to prepare for a new release. Bump the version number in `testing/mozbase/mozversion/setup.py` to "3.0.0", and update the classifiers so they reflect the versions of Python that we now support.

Finally, to prepare a universal distribution, create a `testing/mozbase/mozversion/setup.cfg` file with the following contents:

```
[bdist_wheel]
universal=1
```

When your patch(es) are ready, you will need to push them for review. We are use MozReview for this, and instructions for how to prepare your machine and how to structure your commit messages can be found in our guide: https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html

There may be some review issues to address, but once your contribution has been approved and subsequently landed, we will release the new version of the package!

Comment 1

[Preeti[:preeti]]

Hello everyone! I am new to Bugzilla and would like to work on this bug. May I be assigned this bug, in case it is not assigned to anyone?

I was trying to install mercurial on my laptop while I faced the following error message. Can someone please help me out? Thanks in advance!


The error message:

running build_scripts
creating build/scripts-3.6
copying and adjusting hg -> build/scripts-3.6
changing mode of build/scripts-3.6/hg from 644 to 755
python setup.py  install --root="/" --prefix="/usr/local" --force
running install
running build
running build_mo
running build_py
running build_ext
running build_scripts
running install_lib
creating /usr/local/lib/python3.6/site-packages
error: could not create '/usr/local/lib/python3.6/site-packages': Permission denied
Makefile:81: recipe for target 'install-bin' failed
make: *** [install-bin] Error 1

Comment 2

[Manish [:manishkk]]

(In reply to Preeti from comment #1)
> Hello everyone! I am new to Bugzilla and would like to work on this bug. May
> I be assigned this bug, in case it is not assigned to anyone?
> 
post these queries in IRC #Introduction channel

> I was trying to install mercurial on my laptop while I faced the following
> error message. Can someone please help me out? Thanks in advance!
> 
> 
> The error message:
> 
> running build_scripts
> creating build/scripts-3.6
> copying and adjusting hg -> build/scripts-3.6
> changing mode of build/scripts-3.6/hg from 644 to 755
> python setup.py  install --root="/" --prefix="/usr/local" --force
> running install
> running build
> running build_mo
> running build_py
> running build_ext
> running build_scripts
> running install_lib
> creating /usr/local/lib/python3.6/site-packages
> error: could not create '/usr/local/lib/python3.6/site-packages': Permission
> denied
> Makefile:81: recipe for target 'install-bin' failed
> make: *** [install-bin] Error 1

Comment 3

[Preeti[:preeti]]

I have set up the mercurial and have python 2.7 and Anaconda (python =3.6.5) installed previously. Can someone please guide me on how to proceed further? Thanks in advance!

Comment 4

[Manish [:manishkk]]

This bug depends on some other bug also, first work on them then only you can work on this.

you can look for more good-first-bug here- 
https://www.joshmatthews.net/bugsahoy/?unowned=1&simple=1

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Manish Kumar [:manishkk] from comment #4)
> This bug depends on some other bug also, first work on them then only you
> can work on this.

The dependent bug is already assigned, and patches have landed. It should be safe to start working on this bug.

(In reply to Preeti from comment #3)
> I have set up the mercurial and have python 2.7 and Anaconda (python =3.6.5)
> installed previously. Can someone please guide me on how to proceed further?
> Thanks in advance!

Comment 0 contains details of how to enable and run the tests on Python 3. If you hit any problems or have any specific questions, feel free to ask them here.

Comment 6

[Preeti[:preeti]]

> (In reply to Preeti from comment #3)
> > I have set up the mercurial and have python 2.7 and Anaconda (python =3.6.5)
> > installed previously. Can someone please guide me on how to proceed further?
> > Thanks in advance!
> 
> Comment 0 contains details of how to enable and run the tests on Python 3.
> If you hit any problems or have any specific questions, feel free to ask
> them here.

I have cloned the unified repository to my local disc. Now, I  need to convert all the  python=2.7 programs to python =3 and push the changes to mercurial. Is this precisely what I need to do?

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #6)
> > (In reply to Preeti from comment #3)
> > > I have set up the mercurial and have python 2.7 and Anaconda (python =3.6.5)
> > > installed previously. Can someone please guide me on how to proceed further?
> > > Thanks in advance!
> > 
> > Comment 0 contains details of how to enable and run the tests on Python 3.
> > If you hit any problems or have any specific questions, feel free to ask
> > them here.
> 
> I have cloned the unified repository to my local disc. Now, I  need to
> convert all the  python=2.7 programs to python =3 and push the changes to
> mercurial. Is this precisely what I need to do?

No, you only need to add Python 3 support to mozversion, whilst retaining support for Python 2. Please read comment 0 for details on running the tests against different versions of Python and enabling the tests for Python 3.

Comment 8

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #0)
> +++ This bug was initially created as a clone of Bug #1427568 +++
> 
> We need to add python 3 support to mozversion without losing legacy python
> support. Whilst some aspects were taken care of in bug 1427568 we are now
> able to run the unit tests against Python 3, and many of these are failing.
> 
> To work on this bug you will need to install and configure Mercurial, which
> will enable you to download the Firefox source code. It will also be used to
> commit your changes locally and prepare a patch for review. See the
> installation guide at
> https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> installing.html for help getting Mercurial on your system. Once installed,
> there are some extensions we recommend installing, details of these can be
> found here:
> https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> extensions.html
> 
> To clone the Firefox source code we recommend using the unified repository.
> Details of this and how to create a bookmark for your work can be found
> here:
> https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> unifiedrepo.html
> 
> As this bug relates to Python 3, you will need to have this installed on
> your system. Our continuous integration is currently using Python 3.5, so
> for best results we recommend using the same version locally. There are a
> number of ways to install Python, and they vary depending on your
> environment. We suggest reading over the guides at
> http://docs.python-guide.org/en/latest/starting/installation/#python-3-
> installation-guides to find the best method for you. Note that we also need
> to maintain support for Python 2, so you'll also need to have Python 2.7
> installed.
> 
> Whilst we're moving towards adding support for Python 3, we've disabled any
> tests that fail against this version. This means that in order to work on
> this bug you will need to enable the tests. This can be done by removing
> "skip-if = python == 3" from the manifest file in
> `testing/mozbase/mozversion/tests/manifest.ini`
> 
> To run the tests against Python 3, execute the following command:
> 
> ```
> mach python-test --python=3.5 testing/mozbase/mozversion
> ```
> 
> Work through the test failures, and update the tests and source code for the
> package to simultaneously support Python 2.7 and Python 3.5. You can always
> check that your changes have not inadvertently broken support for Python 2
> by using `--python=2.7` on the command line. 
> 
> If you're new to Python 3, the guide at
> https://docs.python.org/3/howto/pyporting.html may help you to get started
> with understanding the differences. To assist with supporting both Python 2
> and 3, we have vendored the "six" package. You can read more about this and
> how to use it at https://pythonhosted.org/six/
> 
> Note that this package depends on others that may not yet support Python 3.
> If these dependencies are preventing tests from passing, then we can block
> this bug on getting support in those packages. If there are circular
> dependencies then we may need to coordinate landing a sequence of patches
> between bugs.
> 
> Once the package supports Python 3 and the tests pass, we will also need to
> prepare for a new release. Bump the version number in
> `testing/mozbase/mozversion/setup.py` to "3.0.0", and update the classifiers
> so they reflect the versions of Python that we now support.
> 
> Finally, to prepare a universal distribution, create a
> `testing/mozbase/mozversion/setup.cfg` file with the following contents:
> 
> ```
> [bdist_wheel]
> universal=1
> ```
> 
> When your patch(es) are ready, you will need to push them for review. We are
> use MozReview for this, and instructions for how to prepare your machine and
> how to structure your commit messages can be found in our guide:
> https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-
> user.html
> 
> There may be some review issues to address, but once your contribution has
> been approved and subsequently landed, we will release the new version of
> the package!

While running the command for testing, I faced the following error:
preeti@preeti-Inspiron-5577:~/mozilla-unified$ mach python-test --python=3.5 testing/mozbase/mozversion

Command 'mach' not found, did you mean:

  command 'mash' from deb mash
  command 'match' from deb mailavenger

Try: sudo apt install <deb name>

I Googled the error and came accross this link:    https://devguide.python.org/runtests/

May I follow the command (./python -m test -v <filename>) as mentioned in the link?

I am using Ubuntu 18.04

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #8)
> May I follow the command (./python -m test -v <filename>) as mentioned in
> the link?

No, you will need to use mach. Can you try running with an initial ./ such as:

> ./mach python-test --python=3.5 testing/mozbase/mozversion

Comment 10

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #9)
> (In reply to Preeti from comment #8)
> > May I follow the command (./python -m test -v <filename>) as mentioned in
> > the link?
> 
> No, you will need to use mach. Can you try running with an initial ./ such
> as:
> 
> > ./mach python-test --python=3.5 testing/mozbase/mozversion

I am getting the following output:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
It looks like you passed an unrecognized argument into mach.

The python-test command does not accept the arguments: --python=3.5

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #10)
> The python-test command does not accept the arguments: --python=3.5

This would suggest you don't have the latest source code. Can you try running the following:

> hg up central
> hg pull --update

Can you then try the `mach` command again? If it's still failing, please paste the output from `hg summary`.

Comment 12

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #11)
> (In reply to Preeti from comment #10)
> > The python-test command does not accept the arguments: --python=3.5
> 
> This would suggest you don't have the latest source code. Can you try
> running the following:
> 
> > hg up central
> > hg pull --update
> 
> Can you then try the `mach` command again? If it's still failing, please
> paste the output from `hg summary`.
I ran the 2 commands.
It is still showing the same error on the mach command and the output from hg summary is:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg summary
parent: 476204:79c65f8a1a07 
 Bug 1475650: Use llvm-lib when linking with lld-link. r=froydnj
branch: default
warning: Watchman unavailable: get-sockname error: A non-recoverable condition has triggered.  Watchman needs your help!
The triggering condition was at timestamp=1531752230: inotify-add-watch(/home/preeti/HG/accessible/tests/mochitest/treeupdate) -> The user limit on the total number of inotify watches was reached; increase the fs.inotify.max_user_watches sysctl
All requests will continue to fail with this message until you resolve
the underlying problem.  You will find more information on fixing this at
https://facebook.github.io/watchman/docs/troubleshooting.html#poison-inotify-add-watch

commit: (clean)
update: 49482 new changesets, 12 branch heads (merge)

Comment 13

[Dave Hunt [:davehunt] [he/him] ⌚BST]

It looks like you have an up to date revision, and the `--python` command line option should be available to you. Does it work without `--python=3.5` in your command?

Could you also try running `./mach bootstrap` and following the prompts for a recommended environment setup and tool dependencies. When prompted, you can select "1. Firefox for Desktop Artifact Mode", even though you will not be needing to build Firefox at this time.

Comment 14

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #13)
> It looks like you have an up to date revision, and the `--python` command
> line option should be available to you. Does it work without `--python=3.5`
> in your command?

preeti@preeti-Inspiron-5577:~/mozilla-unified$ mach python-test testing/mozbase/mozversion

Command 'mach' not found, did you mean:

  command 'mash' from deb mash
  command 'match' from deb mailavenger

Try: sudo apt install <deb name>



> 
> Could you also try running `./mach bootstrap` and following the prompts for
> a recommended environment setup and tool dependencies. When prompted, you
> can select "1. Firefox for Desktop Artifact Mode", even though you will not
> be needing to build Firefox at this time.


I had used  python bootstrap.py while setting up and I have already selected "1. Firefox for Desktop Artifact Mode"while installation. I get the following error while using the ./mach command:

preeti@preeti-Inspiron-5577:~$ whereis bootstrap.py
bootstrap:
preeti@preeti-Inspiron-5577:~$ ./mach bootstrap.py
bash: ./mach: No such file or directory

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #14)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #13)
> > It looks like you have an up to date revision, and the `--python` command
> > line option should be available to you. Does it work without `--python=3.5`
> > in your command?
> 
> preeti@preeti-Inspiron-5577:~/mozilla-unified$ mach python-test
> testing/mozbase/mozversion
> 
> Command 'mach' not found, did you mean:

You're missing the leading ./

> > Could you also try running `./mach bootstrap` and following the prompts for
> > a recommended environment setup and tool dependencies. When prompted, you
> > can select "1. Firefox for Desktop Artifact Mode", even though you will not
> > be needing to build Firefox at this time.
> 
> 
> I had used  python bootstrap.py while setting up and I have already selected
> "1. Firefox for Desktop Artifact Mode"while installation. I get the
> following error while using the ./mach command:
> 
> preeti@preeti-Inspiron-5577:~$ whereis bootstrap.py
> bootstrap:
> preeti@preeti-Inspiron-5577:~$ ./mach bootstrap.py
> bash: ./mach: No such file or directory

You should be running `./mach bootstrap` from the top source directory. In your case this ia `~/mozilla-unified`.

Comment 16

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #15)
> (In reply to Preeti from comment #14)
> > (In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #13)
> > > It looks like you have an up to date revision, and the `--python` command
> > > line option should be available to you. Does it work without `--python=3.5`
> > > in your command?
> > 
> > preeti@preeti-Inspiron-5577:~/mozilla-unified$ mach python-test
> > testing/mozbase/mozversion
> > 
> > Command 'mach' not found, did you mean:
> 
> You're missing the leading ./
Sorry!
This one worked, do I still need to run `./mach bootstrap`?

Comment 17

[Preeti[:preeti]]

Another set of outputs:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test testing/mozbase/mozversion
New python executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
Also creating executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
Installing setuptools, pip, wheel...done.
running build_ext
building 'psutil._psutil_linux' extension
creating build
creating build/temp.linux-x86_64-2.7
creating build/temp.linux-x86_64-2.7/psutil
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
creating build/lib.linux-x86_64-2.7
creating build/lib.linux-x86_64-2.7/psutil
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
building 'psutil._psutil_posix' extension
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fdebug-prefix-map=/build/python2.7-nbjU53/python2.7-2.7.15~rc1=. -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil

Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
Processing ./third_party/python/certifi
Installing collected packages: certifi
  Running setup.py install for certifi ... done
Successfully installed certifi-2018.4.16
Processing ./third_party/python/pipenv
Installing collected packages: pipenv
  Running setup.py install for pipenv ... done
Successfully installed pipenv-2018.5.18
Processing ./third_party/python/six
Installing collected packages: six
  Running setup.py install for six ... done
Successfully installed six-1.10.0
Processing ./third_party/python/virtualenv
Installing collected packages: virtualenv
  Running setup.py install for virtualenv ... done
Successfully installed virtualenv-15.2.0
Processing ./third_party/python/virtualenv-clone
Building wheels for collected packages: virtualenv-clone
  Running setup.py bdist_wheel for virtualenv-clone ... done
  Stored in directory: /home/preeti/.cache/pip/wheels/8d/a2/bc/0d8a4391b456f926785d5d69bac45b5b1cb246c879cb099139
Successfully built virtualenv-clone
Installing collected packages: virtualenv-clone
Successfully installed virtualenv-clone-0.3.0
Creating a virtualenv for this project…
Using /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7 (2.7.15) to create virtualenv…
⠋Already using interpreter /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
Using real prefix '/usr'
New python executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-2.7/bin/python2.7
Also creating executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-2.7/bin/python
Installing setuptools, pip, wheel...done.

Virtualenv location: /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-2.7
running build_ext
copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil

Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
cat: backend.TestManifestBackend.in: No such file or directory
Build configuration changed. Regenerating backend.
No build detected, test metadata may be incomplete.
No handlers could be found for logger "mozbuild.frontend.reader"
 0:35.75 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_apk.py
 0:36.14 ============================= test session starts ==============================
 0:36.14 platform linux2 -- Python 2.7.15rc1, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-2.7/bin/python
 0:36.14 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:36.14 collecting ... collected 2 items
 0:36.14 
 0:36.14 testing/mozbase/mozversion/tests/test_apk.py::test_basic PASSED          [ 50%]
 0:36.14 testing/mozbase/mozversion/tests/test_apk.py::test_with_package_name PASSED [100%]
 0:36.15 
 0:36.15 =========================== 2 passed in 0.01 seconds ===========================
 0:36.20 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:37.13 ============================= test session starts ==============================
 0:37.13 platform linux2 -- Python 2.7.15rc1, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-2.7/bin/python
 0:37.13 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:37.14 collecting ... collected 11 items
 0:37.15 
 0:37.23 testing/mozbase/mozversion/tests/test_binary.py::test_real_binary SKIPPED [  9%]
 0:37.23 testing/mozbase/mozversion/tests/test_binary.py::test_binary PASSED      [ 18%]
 0:37.23 testing/mozbase/mozversion/tests/test_binary.py::test_symlinked_binary PASSED [ 27%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_binary_in_current_path PASSED [ 36%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_with_ini_files_on_osx PASSED [ 45%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_invalid_binary_path PASSED [ 54%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_without_ini_files PASSED [ 63%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_without_platform_ini_file PASSED [ 72%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_without_application_ini_file PASSED [ 81%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_with_exe PASSED    [ 90%]
 0:37.24 testing/mozbase/mozversion/tests/test_binary.py::test_not_found_with_binary_specified PASSED [100%]
 0:37.24 
 0:37.24 ===================== 10 passed, 1 skipped in 0.12 seconds =====================
 0:37.30 Return code from mach python-test: 0







preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
Creating a virtualenv for this project…
Using /usr/bin/python3.5m (3.5.5) to create virtualenv…
⠋Running virtualenv with interpreter /usr/bin/python3.5m
Using base prefix '/usr'
New python executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python3.5m
Also creating executable in /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python
Installing setuptools, pip, wheel...done.

Virtualenv location: /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5
Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
 0:06.61 TEST-UNEXPECTED-FAIL | No tests collected (Not in PYTHON_UNITTEST_MANIFESTS?)


Could you please guide me on how to proceed now?

Comment 18

[Dave Hunt [:davehunt] [he/him] ⌚BST]

As you'll see from your output those tests were running against Python 2.7, and you will need to be able to run them against Python 3.5. I'm concerned that the `--python` command line option is not being recognised. Can you paste the output from `./mach python-test --help`. I would still also suggest running `./mach bootstrap` if you have not.

So I can guide you best, can you tell me what your level of experience of Python is?

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Nevermind the first part of my last comment, I missed that your comment also included a working Python 3.5 command. Please refer to comment 0 for how to remove the skip-if and enable the tests for Python 3.5.

Comment 20

[Preeti[:preeti]]

> So I can guide you best, can you tell me what your level of experience of
> Python is?

I have been coding in python for the last 3 months and this bug is my first bug in open source as well as Bugzilla.

Comment 21

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #19)
> Nevermind the first part of my last comment, I missed that your comment also
> included a working Python 3.5 command. Please refer to comment 0 for how to
> remove the skip-if and enable the tests for Python 3.5.

preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
Build configuration changed. Regenerating backend.
No build detected, test metadata may be incomplete.
No handlers could be found for logger "mozbuild.frontend.reader"
 0:30.38 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_apk.py
 0:32.17 ============================= test session starts ==============================
 0:32.17 platform linux -- Python 3.5.5, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python
 0:32.17 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:32.17 collecting ... collected 2 items
 0:32.19 
 0:32.29 testing/mozbase/mozversion/tests/test_apk.py::test_basic FAILED          [ 50%]
 0:32.33 testing/mozbase/mozversion/tests/test_apk.py::test_with_package_name FAILED [100%]
 0:32.33 
 0:32.33 =================================== FAILURES ===================================
 0:32.33 __________________________________ test_basic __________________________________
 0:32.33 
 0:32.33 apk = '/tmp/pytest-of-preeti/pytest-0/test_basic0/apk.zip'
 0:32.33 
 0:32.33     def test_basic(apk):
 0:32.33 >       v = get_version(apk)
 0:32.34 
 0:32.34 testing/mozbase/mozversion/tests/test_apk.py:36:
 0:32.34 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:122: in get_version
 0:32.34     version = LocalFennecVersion(binary)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:58: in __init__
 0:32.34     self.get_gecko_info(path)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:67: in get_gecko_info
 0:32.34     section)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:39: in _parse_ini_file
 0:32.34     config.readfp(fp)
 0:32.34 /usr/lib/python3.5/configparser.py:760: in readfp
 0:32.34     self.read_file(fp, source=filename)
 0:32.34 /usr/lib/python3.5/configparser.py:715: in read_file
 0:32.34     self._read(f, source)
 0:32.34 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:32.34 
 0:32.34 self = <configparser.RawConfigParser object at 0x7efead9be588>
 0:32.34 fp = <zipfile.ZipExtFile name='application.ini' mode='r'>
 0:32.34 fpname = 'application.ini'
 0:32.34 
 0:32.34     def _read(self, fp, fpname):
 0:32.34         """Parse a sectioned configuration file.
 0:32.34     
 0:32.34             Each section in a configuration file contains a header, indicated by
 0:32.34             a name in square brackets (`[]'), plus key/value options, indicated by
 0:32.34             `name' and `value' delimited with a specific substring (`=' or `:' by
 0:32.34             default).
 0:32.34     
 0:32.34             Values can span multiple lines, as long as they are indented deeper
 0:32.34             than the first line of the value. Depending on the parser's mode, blank
 0:32.34             lines may be treated as parts of multiline values or ignored.
 0:32.34     
 0:32.34             Configuration files may include comments, prefixed by specific
 0:32.34             characters (`#' and `;' by default). Comments may appear on their own
 0:32.34             in an otherwise empty line or may be entered in lines holding values or
 0:32.34             section names.
 0:32.34             """
 0:32.34         elements_added = set()
 0:32.34         cursect = None                        # None, or a dictionary
 0:32.34         sectname = None
 0:32.34         optname = None
 0:32.34         lineno = 0
 0:32.34         indent_level = 0
 0:32.34         e = None                              # None, or an exception
 0:32.34         for lineno, line in enumerate(fp, start=1):
 0:32.34             comment_start = sys.maxsize
 0:32.34             # strip inline comments
 0:32.34             inline_prefixes = {p: -1 for p in self._inline_comment_prefixes}
 0:32.34             while comment_start == sys.maxsize and inline_prefixes:
 0:32.34                 next_prefixes = {}
 0:32.34                 for prefix, index in inline_prefixes.items():
 0:32.34                     index = line.find(prefix, index+1)
 0:32.34                     if index == -1:
 0:32.34                         continue
 0:32.34                     next_prefixes[prefix] = index
 0:32.34                     if index == 0 or (index > 0 and line[index-1].isspace()):
 0:32.34                         comment_start = min(comment_start, index)
 0:32.34                 inline_prefixes = next_prefixes
 0:32.34             # strip full line comments
 0:32.34             for prefix in self._comment_prefixes:
 0:32.34 >               if line.strip().startswith(prefix):
 0:32.34 E               TypeError: startswith first arg must be bytes or a tuple of bytes, not str
 0:32.34 
 0:32.34 /usr/lib/python3.5/configparser.py:1028: TypeError
 0:32.34 ____________________________ test_with_package_name ____________________________
 0:32.34 
 0:32.34 apk = '/tmp/pytest-of-preeti/pytest-0/test_with_package_name0/apk.zip'
 0:32.34 
 0:32.34     def test_with_package_name(apk):
 0:32.34         with zipfile.ZipFile(apk, 'a') as z:
 0:32.34             z.writestr('package-name.txt', 'org.mozilla.fennec')
 0:32.34 >       v = get_version(apk)
 0:32.34 
 0:32.34 testing/mozbase/mozversion/tests/test_apk.py:44:
 0:32.34 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:122: in get_version
 0:32.34     version = LocalFennecVersion(binary)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:58: in __init__
 0:32.34     self.get_gecko_info(path)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:67: in get_gecko_info
 0:32.34     section)
 0:32.34 testing/mozbase/mozversion/mozversion/mozversion.py:39: in _parse_ini_file
 0:32.34     config.readfp(fp)
 0:32.34 /usr/lib/python3.5/configparser.py:760: in readfp
 0:32.34     self.read_file(fp, source=filename)
 0:32.34 /usr/lib/python3.5/configparser.py:715: in read_file
 0:32.34     self._read(f, source)
 0:32.34 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:32.34 
 0:32.34 self = <configparser.RawConfigParser object at 0x7efead9ca710>
 0:32.34 fp = <zipfile.ZipExtFile name='application.ini' mode='r'>
 0:32.34 fpname = 'application.ini'
 0:32.34 
 0:32.34     def _read(self, fp, fpname):
 0:32.34         """Parse a sectioned configuration file.
 0:32.34     
 0:32.34             Each section in a configuration file contains a header, indicated by
 0:32.34             a name in square brackets (`[]'), plus key/value options, indicated by
 0:32.34             `name' and `value' delimited with a specific substring (`=' or `:' by
 0:32.34             default).
 0:32.34     
 0:32.34             Values can span multiple lines, as long as they are indented deeper
 0:32.34             than the first line of the value. Depending on the parser's mode, blank
 0:32.34             lines may be treated as parts of multiline values or ignored.
 0:32.34     
 0:32.34             Configuration files may include comments, prefixed by specific
 0:32.34             characters (`#' and `;' by default). Comments may appear on their own
 0:32.34             in an otherwise empty line or may be entered in lines holding values or
 0:32.34             section names.
 0:32.34             """
 0:32.34         elements_added = set()
 0:32.34         cursect = None                        # None, or a dictionary
 0:32.34         sectname = None
 0:32.34         optname = None
 0:32.34         lineno = 0
 0:32.34         indent_level = 0
 0:32.34         e = None                              # None, or an exception
 0:32.34         for lineno, line in enumerate(fp, start=1):
 0:32.34             comment_start = sys.maxsize
 0:32.34             # strip inline comments
 0:32.34             inline_prefixes = {p: -1 for p in self._inline_comment_prefixes}
 0:32.34             while comment_start == sys.maxsize and inline_prefixes:
 0:32.34                 next_prefixes = {}
 0:32.34                 for prefix, index in inline_prefixes.items():
 0:32.34                     index = line.find(prefix, index+1)
 0:32.34                     if index == -1:
 0:32.34                         continue
 0:32.34                     next_prefixes[prefix] = index
 0:32.34                     if index == 0 or (index > 0 and line[index-1].isspace()):
 0:32.34                         comment_start = min(comment_start, index)
 0:32.34                 inline_prefixes = next_prefixes
 0:32.34             # strip full line comments
 0:32.34             for prefix in self._comment_prefixes:
 0:32.34 >               if line.strip().startswith(prefix):
 0:32.34 E               TypeError: startswith first arg must be bytes or a tuple of bytes, not str
 0:32.34 
 0:32.34 /usr/lib/python3.5/configparser.py:1028: TypeError
 0:32.34 =========================== 2 failed in 0.16 seconds ===========================
 0:32.37 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:32.37 Setting retcode to 1 from /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_apk.py
 0:33.75 Traceback (most recent call last):
 0:33.75   File "/home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py", line 15, in <module>
 0:33.75     from moztest.selftest.fixtures import binary as real_binary  # noqa: F401
 0:33.76   File "/home/preeti/mozilla-unified/testing/mozbase/moztest/moztest/selftest/fixtures.py", line 20, in <module>
 0:33.76     from mozbuild.base import MozbuildObject
 0:33.76   File "/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py", line 14, in <module>
 0:33.76     import which
 0:33.76   File "/home/preeti/mozilla-unified/third_party/python/which/which.py", line 284
 0:33.76     except getopt.GetoptError, msg:
 0:33.76                              ^
 0:33.76 SyntaxError: invalid syntax
 0:33.82 TEST-UNEXPECTED-FAIL | No test output (missing mozunit.main() call?): /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:33.83 Return code from mach python-test: 1

I have made changes as you had mentioned in Comment 0. Now what do I do? I am ready to learn new technologies that are required to fix this bug.

Comment 22

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Excellent, you have the tests running in Python 3! Now we need to add support for Python 3 without affecting the package under Python 2. There are two distinct exceptions in the tests you ran.

The first is from the call to `ConfigParser.readfp` on line 39 of testing/mozbase/mozversion/mozversion/mozversion.py. This is failing with "TypeError: startswith first arg must be bytes or a tuple of bytes, not str". I would suggest taking a look at the changes to ConfigParser between Python 2 and Python 3. It's also worth noting that the code path that's failing is using `ZipFile`, so it could also be related to changes there. I suspect the issue is related to unicode strings vs byte strings, the behaviour of which has changed in Python 3.

The second is invalid syntax for catching an exception. This is easy to fix (just change `except [exception] as error`), however the code in question is actually a vendored copy of the which package [1]. Ideally when the Python 3 incompatibility is in a third-party package we'd simply update to the latest version, however we already have the latest version of this package. In this case, I would suggest using Python 3's shutil.which [2] when it's available, and falling back to the third-party package when it's not.

[1] https://github.com/trentm/which
[2] https://docs.python.org/3.5/library/shutil.html#shutil.which

Comment 23

[Preeti[:preeti]]

As you mentioned that the second part is easier to fix, I want to  consider this one first:

> The second is invalid syntax for catching an exception. This is easy to fix
> (just change `except [exception] as error`),

But, I didn't find any line with try-exception in mozversion.py.I guess, I am missing out somewhere. So could you please elaborate? 


 however the code in question is
> actually a vendored copy of the which package [1]. Ideally when the Python 3
> incompatibility is in a third-party package we'd simply update to the latest
> version, however we already have the latest version of this package. In this
> case, I would suggest using Python 3's shutil.which [2] when it's available,
> and falling back to the third-party package when it's not.
> 
> [1] https://github.com/trentm/which
> [2] https://docs.python.org/3.5/library/shutil.html#shutil.which

This means that I need to add "from shutil import while" and later call shutil.while("python=3.5.5") in the mozversion.py file.However, I am unable to implement some method (if else or switch statement) so that first the program tries to execute with the python file as given by the shutil.while location, or use the while.py file to execute the program if the python file is absent.

This is the first time that I am using High-level file operations, so please excuse me for my doubts!

Comment 24

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #23)
> As you mentioned that the second part is easier to fix, I want to  consider
> this one first:
> 
> > The second is invalid syntax for catching an exception. This is easy to fix
> > (just change `except [exception] as error`),
> 
> But, I didn't find any line with try-exception in mozversion.py.I guess, I
> am missing out somewhere. So could you please elaborate? 

If you follow the traceback you should see that the `except` line is in `/home/preeti/mozilla-unified/third_party/python/which/which.py` and not `mozversion.py`. Follow the traceback further and you'll see that it's actually `/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py` that attempts to import `which`.

>  however the code in question is
> > actually a vendored copy of the which package [1]. Ideally when the Python 3
> > incompatibility is in a third-party package we'd simply update to the latest
> > version, however we already have the latest version of this package. In this
> > case, I would suggest using Python 3's shutil.which [2] when it's available,
> > and falling back to the third-party package when it's not.
> > 
> > [1] https://github.com/trentm/which
> > [2] https://docs.python.org/3.5/library/shutil.html#shutil.which
> 
> This means that I need to add "from shutil import while" and later call
> shutil.while("python=3.5.5") in the mozversion.py file.However, I am unable
> to implement some method (if else or switch statement) so that first the
> program tries to execute with the python file as given by the shutil.while
> location, or use the while.py file to execute the program if the python file
> is absent.
> 
> This is the first time that I am using High-level file operations, so please
> excuse me for my doubts!

You should be able to modify the import in `/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py` to attempt to use shutil.which, and fallback to the third-party package when it's not available (Python 2). Something like the following could work:

> try:
>     from shutil import which
> except ImportError:
>     # shutil.which is not available in Python 2.7
>     import which

This will mean the code can continue to call `which()`, however the module used will differ depending on the version of Python in use.

Comment 25

[Preeti[:preeti]]

> If you follow the traceback you should see that the `except` line is in
> `/home/preeti/mozilla-unified/third_party/python/which/which.py` and not
> `mozversion.py`. Follow the traceback further and you'll see that it's
> actually `/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py`
> that attempts to import `which`.

I was trying to fix the which.py function by changing the exception statement(which was the only error displayed till then)  to "except (getopt.GetoptError, msg) , e:", it did not work, so I changed it to "except (getopt.GetoptError, msg) as e:" as mentioned in [1] and it worked but then it started showing error for print statement syntax (using parentheses in python 3 and it is not allowed in python 2). However after fixing these, a new set of error pops up:

E0401:Unable to import '_winreg'
E1101:Generator 'generator' has no 'next' member
E0602:Undefined variable 'msg'
E0602:Undefined variable 'msg'
W0120:Else clause on loop without a break statement
W0622:Redefining built-in 'all'
W0612:Unused variable 'e'

Again in '/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py' now it is showing an error that it is unable to import which.

[1]  https://stackoverflow.com/questions/6470428/catch-multiple-exceptions-in-one-line-except-block

Comment 26

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #25)
> > If you follow the traceback you should see that the `except` line is in
> > `/home/preeti/mozilla-unified/third_party/python/which/which.py` and not
> > `mozversion.py`. Follow the traceback further and you'll see that it's
> > actually `/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py`
> > that attempts to import `which`.
> 
> I was trying to fix the which.py function by changing the exception
> statement(which was the only error displayed till then)  to "except
> (getopt.GetoptError, msg) , e:", it did not work, so I changed it to "except
> (getopt.GetoptError, msg) as e:" as mentioned in [1] and it worked but then
> it started showing error for print statement syntax (using parentheses in
> python 3 and it is not allowed in python 2). However after fixing these, a
> new set of error pops up:

There is only one exception we're catching here, so there's no need to use paranthesis. If we're going to modify `which.py` then we should first make it clear somewhere that we have patched the third-party package. I'm not sure where it would be best to do this. Did you try my suggestion of using `shutil.which` on Python 3 instead of patching `which.py`?

To correctly fix the exception handling in `which.py` you'd want something along the lines of:

> except getopt.GetoptError as e:

You'll then need to replace the uses of `msg` with `e`.

> E0401:Unable to import '_winreg'
> E1101:Generator 'generator' has no 'next' member
> E0602:Undefined variable 'msg'
> E0602:Undefined variable 'msg'
> W0120:Else clause on loop without a break statement
> W0622:Redefining built-in 'all'
> W0612:Unused variable 'e'

_winreg should only be imported when on Windows, yet you previously mentioned that you're using Ubuntu. The undefined 'msg' variables are related to the exception handling as mentioned above.

> Again in '/home/preeti/mozilla-unified/python/mozbuild/mozbuild/base.py' now
> it is showing an error that it is unable to import which.

Please try using `shutil.which` as mentioned in comment 24 before proceeding with the patch for `which.py` to see if that works. If not, we may need to proceed as you have started here for now.

I would not be surprised if there are a few other Python 3 issues beyond the use of `which.py`, and you may find this to be a bit of a rabbit-hole. If it seems there are too many layers then we may be able to split this into a smaller bugs and tackle them individually.

Comment 27

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #26)

> Please try using `shutil.which` as mentioned in comment 24 before proceeding
> with the patch for `which.py` to see if that works. If not, we may need to
> proceed as you have started here for now.
 
I have incorporated the shutil.which command as mentioned in comment 24 in base.py , corrected the except expression and removed the additional parenthesis that I had applied on seeing the error message.
Now  below is my output and please tell me  how I should proceed now:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
Build configuration changed. Regenerating backend.
No build detected, test metadata may be incomplete.
No handlers could be found for logger "mozbuild.frontend.reader"
 0:04.59 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_apk.py
 0:04.95 ============================= test session starts ==============================
 0:04.95 platform linux -- Python 3.5.5, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python
 0:04.95 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:04.95 collecting ... collected 2 items
 0:04.96 
 0:05.03 testing/mozbase/mozversion/tests/test_apk.py::test_basic FAILED          [ 50%]
 0:05.09 testing/mozbase/mozversion/tests/test_apk.py::test_with_package_name FAILED [100%]
 0:05.09 
 0:05.09 =================================== FAILURES ===================================
 0:05.09 __________________________________ test_basic __________________________________
 0:05.09 
 0:05.09 apk = '/tmp/pytest-of-preeti/pytest-2/test_basic0/apk.zip'
 0:05.09 
 0:05.09     def test_basic(apk):
 0:05.09 >       v = get_version(apk)
 0:05.09 
 0:05.09 testing/mozbase/mozversion/tests/test_apk.py:36:
 0:05.09 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:122: in get_version
 0:05.09     version = LocalFennecVersion(binary)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:58: in __init__
 0:05.09     self.get_gecko_info(path)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:67: in get_gecko_info
 0:05.09     section)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:39: in _parse_ini_file
 0:05.09     config.readfp(fp)
 0:05.09 /usr/lib/python3.5/configparser.py:760: in readfp
 0:05.09     self.read_file(fp, source=filename)
 0:05.09 /usr/lib/python3.5/configparser.py:715: in read_file
 0:05.09     self._read(f, source)
 0:05.09 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:05.09 
 0:05.09 self = <configparser.RawConfigParser object at 0x7f57e5d41978>
 0:05.09 fp = <zipfile.ZipExtFile name='application.ini' mode='r'>
 0:05.09 fpname = 'application.ini'
 0:05.09 
 0:05.09     def _read(self, fp, fpname):
 0:05.09         """Parse a sectioned configuration file.
 0:05.09     
 0:05.09             Each section in a configuration file contains a header, indicated by
 0:05.09             a name in square brackets (`[]'), plus key/value options, indicated by
 0:05.09             `name' and `value' delimited with a specific substring (`=' or `:' by
 0:05.09             default).
 0:05.09     
 0:05.09             Values can span multiple lines, as long as they are indented deeper
 0:05.09             than the first line of the value. Depending on the parser's mode, blank
 0:05.09             lines may be treated as parts of multiline values or ignored.
 0:05.09     
 0:05.09             Configuration files may include comments, prefixed by specific
 0:05.09             characters (`#' and `;' by default). Comments may appear on their own
 0:05.09             in an otherwise empty line or may be entered in lines holding values or
 0:05.09             section names.
 0:05.09             """
 0:05.09         elements_added = set()
 0:05.09         cursect = None                        # None, or a dictionary
 0:05.09         sectname = None
 0:05.09         optname = None
 0:05.09         lineno = 0
 0:05.09         indent_level = 0
 0:05.09         e = None                              # None, or an exception
 0:05.09         for lineno, line in enumerate(fp, start=1):
 0:05.09             comment_start = sys.maxsize
 0:05.09             # strip inline comments
 0:05.09             inline_prefixes = {p: -1 for p in self._inline_comment_prefixes}
 0:05.09             while comment_start == sys.maxsize and inline_prefixes:
 0:05.09                 next_prefixes = {}
 0:05.09                 for prefix, index in inline_prefixes.items():
 0:05.09                     index = line.find(prefix, index+1)
 0:05.09                     if index == -1:
 0:05.09                         continue
 0:05.09                     next_prefixes[prefix] = index
 0:05.09                     if index == 0 or (index > 0 and line[index-1].isspace()):
 0:05.09                         comment_start = min(comment_start, index)
 0:05.09                 inline_prefixes = next_prefixes
 0:05.09             # strip full line comments
 0:05.09             for prefix in self._comment_prefixes:
 0:05.09 >               if line.strip().startswith(prefix):
 0:05.09 E               TypeError: startswith first arg must be bytes or a tuple of bytes, not str
 0:05.09 
 0:05.09 /usr/lib/python3.5/configparser.py:1028: TypeError
 0:05.09 ____________________________ test_with_package_name ____________________________
 0:05.09 
 0:05.09 apk = '/tmp/pytest-of-preeti/pytest-2/test_with_package_name0/apk.zip'
 0:05.09 
 0:05.09     def test_with_package_name(apk):
 0:05.09         with zipfile.ZipFile(apk, 'a') as z:
 0:05.09             z.writestr('package-name.txt', 'org.mozilla.fennec')
 0:05.09 >       v = get_version(apk)
 0:05.09 
 0:05.09 testing/mozbase/mozversion/tests/test_apk.py:44:
 0:05.09 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:122: in get_version
 0:05.09     version = LocalFennecVersion(binary)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:58: in __init__
 0:05.09     self.get_gecko_info(path)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:67: in get_gecko_info
 0:05.09     section)
 0:05.09 testing/mozbase/mozversion/mozversion/mozversion.py:39: in _parse_ini_file
 0:05.09     config.readfp(fp)
 0:05.09 /usr/lib/python3.5/configparser.py:760: in readfp
 0:05.09     self.read_file(fp, source=filename)
 0:05.09 /usr/lib/python3.5/configparser.py:715: in read_file
 0:05.09     self._read(f, source)
 0:05.09 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 0:05.09 
 0:05.09 self = <configparser.RawConfigParser object at 0x7f57e5d54668>
 0:05.09 fp = <zipfile.ZipExtFile name='application.ini' mode='r'>
 0:05.09 fpname = 'application.ini'
 0:05.09 
 0:05.09     def _read(self, fp, fpname):
 0:05.09         """Parse a sectioned configuration file.
 0:05.09     
 0:05.09             Each section in a configuration file contains a header, indicated by
 0:05.09             a name in square brackets (`[]'), plus key/value options, indicated by
 0:05.09             `name' and `value' delimited with a specific substring (`=' or `:' by
 0:05.09             default).
 0:05.09     
 0:05.09             Values can span multiple lines, as long as they are indented deeper
 0:05.09             than the first line of the value. Depending on the parser's mode, blank
 0:05.09             lines may be treated as parts of multiline values or ignored.
 0:05.09     
 0:05.09             Configuration files may include comments, prefixed by specific
 0:05.10             characters (`#' and `;' by default). Comments may appear on their own
 0:05.10             in an otherwise empty line or may be entered in lines holding values or
 0:05.10             section names.
 0:05.10             """
 0:05.10         elements_added = set()
 0:05.10         cursect = None                        # None, or a dictionary
 0:05.10         sectname = None
 0:05.10         optname = None
 0:05.10         lineno = 0
 0:05.10         indent_level = 0
 0:05.10         e = None                              # None, or an exception
 0:05.10         for lineno, line in enumerate(fp, start=1):
 0:05.10             comment_start = sys.maxsize
 0:05.10             # strip inline comments
 0:05.10             inline_prefixes = {p: -1 for p in self._inline_comment_prefixes}
 0:05.10             while comment_start == sys.maxsize and inline_prefixes:
 0:05.10                 next_prefixes = {}
 0:05.10                 for prefix, index in inline_prefixes.items():
 0:05.10                     index = line.find(prefix, index+1)
 0:05.10                     if index == -1:
 0:05.10                         continue
 0:05.10                     next_prefixes[prefix] = index
 0:05.10                     if index == 0 or (index > 0 and line[index-1].isspace()):
 0:05.10                         comment_start = min(comment_start, index)
 0:05.10                 inline_prefixes = next_prefixes
 0:05.10             # strip full line comments
 0:05.10             for prefix in self._comment_prefixes:
 0:05.10 >               if line.strip().startswith(prefix):
 0:05.10 E               TypeError: startswith first arg must be bytes or a tuple of bytes, not str
 0:05.10 
 0:05.10 /usr/lib/python3.5/configparser.py:1028: TypeError
 0:05.10 =========================== 2 failed in 0.14 seconds ===========================
 0:05.15 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:05.15 Setting retcode to 1 from /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_apk.py
 0:05.60 ============================= test session starts ==============================
 0:05.60 platform linux -- Python 3.5.5, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python
 0:05.60 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:05.62 collecting ... collected 11 items
 0:05.62 
 0:05.62 testing/mozbase/mozversion/tests/test_binary.py::test_real_binary SKIPPED [  9%]
 0:05.63 testing/mozbase/mozversion/tests/test_binary.py::test_binary PASSED      [ 18%]
 0:05.63 testing/mozbase/mozversion/tests/test_binary.py::test_symlinked_binary PASSED [ 27%]
 0:05.63 testing/mozbase/mozversion/tests/test_binary.py::test_binary_in_current_path PASSED [ 36%]
 0:05.64 testing/mozbase/mozversion/tests/test_binary.py::test_with_ini_files_on_osx PASSED [ 45%]
 0:05.64 testing/mozbase/mozversion/tests/test_binary.py::test_invalid_binary_path PASSED [ 54%]
 0:05.64 testing/mozbase/mozversion/tests/test_binary.py::test_without_ini_files PASSED [ 63%]
 0:05.64 testing/mozbase/mozversion/tests/test_binary.py::test_without_platform_ini_file PASSED [ 72%]
 0:05.64 testing/mozbase/mozversion/tests/test_binary.py::test_without_application_ini_file PASSED [ 81%]
 0:05.65 testing/mozbase/mozversion/tests/test_binary.py::test_with_exe PASSED    [ 90%]
 0:05.65 testing/mozbase/mozversion/tests/test_binary.py::test_not_found_with_binary_specified PASSED [100%]
 0:05.65 
 0:05.65 ===================== 10 passed, 1 skipped in 0.05 seconds =====================
 0:05.71 Return code from mach python-test: 1

Comment 28

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #27)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #26)
> 
> > Please try using `shutil.which` as mentioned in comment 24 before proceeding
> > with the patch for `which.py` to see if that works. If not, we may need to
> > proceed as you have started here for now.
>  
> I have incorporated the shutil.which command as mentioned in comment 24 in
> base.py , corrected the except expression and removed the additional
> parenthesis that I had applied on seeing the error message.
> Now  below is my output and please tell me  how I should proceed now:

This is great! It looks like you've got 10 of the tests passing just be fixing the issues with importing the third-party package. Just to clarify, with the switch to `shutil.which` you shouldn't need any of your changes to `which.py`.

I think we should land this before following up with the two failures from `test_apk.py`. You can add the `skip-if = python == 3` into `testing/mozbase/mozversion/tests/manifest.ini` but instead of it being in the `[DEFAULT]` section, if you put it under the `[test_apk.py]` section and run the tests again you should see that the failing tests are being skipped.

Once you've done that, you can prepare a patch. Follow the guide at http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install.html to configure MozReview and then read through http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html to learn how to submit a patch for review.

After we've landed the patch to enable those 10 tests, we can then focus on the remaining two failures.

Comment 29

[Preeti[:preeti]]

> > > Please try using `shutil.which` as mentioned in comment 24 before proceeding
> > > with the patch for `which.py` to see if that works. If not, we may need to
> > > proceed as you have started here for now.
> >  
> > I have incorporated the shutil.which command as mentioned in comment 24 in
> > base.py , corrected the except expression and removed the additional
> > parenthesis that I had applied on seeing the error message.
> > Now  below is my output and please tell me  how I should proceed now:
> 
> This is great! It looks like you've got 10 of the tests passing just be
> fixing the issues with importing the third-party package. Just to clarify,
> with the switch to `shutil.which` you shouldn't need any of your changes to
> `which.py`.
> 
> I think we should land this before following up with the two failures from
> `test_apk.py`. You can add the `skip-if = python == 3` into
> `testing/mozbase/mozversion/tests/manifest.ini` but instead of it being in
> the `[DEFAULT]` section, if you put it under the `[test_apk.py]` section and
> run the tests again you should see that the failing tests are being skipped.
I have incorporated these changes into my code.

> 
> Once you've done that, you can prepare a patch. Follow the guide at
> http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/
> install.html to configure MozReview and then read through
> http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/
> commits.html to learn how to submit a patch for review.
> 
This is the output:
preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
 0:06.16 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:08.04 ============================= test session starts ==============================
 0:08.04 platform linux -- Python 3.5.5, pytest-3.6.2, py-1.5.4, pluggy-0.6.0 -- /home/preeti/mozilla-unified/obj-x86_64-pc-linux-gnu/_virtualenvs/mozilla-unified-ymsYRbdn-3.5/bin/python
 0:08.04 rootdir: /home/preeti/mozilla-unified, inifile: /home/preeti/mozilla-unified/config/mozunit/mozunit/pytest.ini
 0:08.06 collecting ... collected 11 items
 0:08.07 
 0:08.07 testing/mozbase/mozversion/tests/test_binary.py::test_real_binary SKIPPED [  9%]
 0:08.09 testing/mozbase/mozversion/tests/test_binary.py::test_binary PASSED      [ 18%]
 0:08.09 testing/mozbase/mozversion/tests/test_binary.py::test_symlinked_binary PASSED [ 27%]
 0:08.09 testing/mozbase/mozversion/tests/test_binary.py::test_binary_in_current_path PASSED [ 36%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_with_ini_files_on_osx PASSED [ 45%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_invalid_binary_path PASSED [ 54%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_without_ini_files PASSED [ 63%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_without_platform_ini_file PASSED [ 72%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_without_application_ini_file PASSED [ 81%]
 0:08.10 testing/mozbase/mozversion/tests/test_binary.py::test_with_exe PASSED    [ 90%]
 0:08.11 testing/mozbase/mozversion/tests/test_binary.py::test_not_found_with_binary_specified PASSED [100%]
 0:08.11 
 0:08.11 ===================== 10 passed, 1 skipped in 0.07 seconds =====================
 0:08.16 Return code from mach python-test: 0



I have been trying to make a patch for the last two days and push it , but I am getting a lot of errors...I am still  trying to get my errors solved  from the #introduction channel. Please do consider and sorry for the delay.

It would nice be great if you could guide me whether I should contact some other other channel to get my problems on mercurial solved.

Comment 30

[Preeti[:preeti]]

Attachment: Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

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

Comment 31

[Preeti[:preeti]]

I have pushed the code and then I get the output:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg push review
pushing to https://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
redirecting push to https://reviewboard-hg.mozilla.org/gecko
searching for changes
no changes found
submitting 1 changesets for review

changeset:  476208:1ddaf36b195a
summary:    Bug 1471920 - [mozversion] Add support for python 3; r?davehunt
review:     https://reviewboard.mozilla.org/r/258604 (draft)

review id:  bz://1471920/preeti
review url: https://reviewboard.mozilla.org/r/258602 (draft)

publish these review requests now (Yn)?  y
(published review request 258602)

On opening the link I get a command like hg pull .....
Do I need to execute that?

Further please guide me on how to proceed now and  work on the remaining test failures.

Comment 32

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review265636

::: commit-message-79c65:1
(Diff revision 1)
> +Bug 1471920 - [mozversion] Add support for python 3; r?davehunt

Please change the commit message to reflect the change. We're not yet at Pyhton 3 support, so perhaps a message like "[mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available"

::: python/mozbuild/mozbuild/base.py:14
(Diff revision 1)
>  import mozpack.path as mozpath
>  import multiprocessing
>  import os
>  import subprocess
>  import sys
> +#import which

Nit: Remove this commented out line.

::: python/mozbuild/mozbuild/base.py:18
(Diff revision 1)
>  import sys
> +#import which
> +try:
> +    from shutil import which
> +except ImportError:
> +# shutil.which is not available in Python 2.7

Nit: Indent this line.

::: third_party/python/which/which.py:284
(Diff revision 1)
>      altpath = None
>      exts = None
>      try:
>          optlist, args = getopt.getopt(argv[1:], 'haVvqp:e:',
>              ['help', 'all', 'version', 'verbose', 'quiet', 'path=', 'exts='])
> -    except getopt.GetoptError, msg:
> +    except getopt.GetoptError as e:

As we're avoiding use of `which.py` when using Python 3, please revert the changes to this file.

Comment 33

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #31)
> I have pushed the code and then I get the output:
> 
> preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg push review
> pushing to https://reviewboard-hg.mozilla.org/autoreview
> searching for appropriate review repository
> redirecting push to https://reviewboard-hg.mozilla.org/gecko
> searching for changes
> no changes found
> submitting 1 changesets for review
> 
> changeset:  476208:1ddaf36b195a
> summary:    Bug 1471920 - [mozversion] Add support for python 3; r?davehunt
> review:     https://reviewboard.mozilla.org/r/258604 (draft)
> 
> review id:  bz://1471920/preeti
> review url: https://reviewboard.mozilla.org/r/258602 (draft)
> 
> publish these review requests now (Yn)?  y
> (published review request 258602)
> 
> On opening the link I get a command like hg pull .....
> Do I need to execute that?
> 
> Further please guide me on how to proceed now and  work on the remaining
> test failures.

Sorry, I missed this question. Your patch was submitted successfully, and I've reviewed it. There are a few changes, but in general it looks good. Once you've addressed my comments we can test this patch to ee if it causes any regressions. If it doesn't then we can land it and progress with the remaining failures.

Comment 34

[Preeti[:preeti]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

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

Comment 35

[Preeti[:preeti]]

> Sorry, I missed this question. Your patch was submitted successfully, and
> I've reviewed it. There are a few changes, but in general it looks good.
> Once you've addressed my comments we can test this patch to ee if it causes
> any regressions. If it doesn't then we can land it and progress with the
> remaining failures.

I have pushed the code again for review, after incorporating the changes you have mentioned in comment 32!

Comment 36

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review266100

Sorry for the delay, I'm at a conference this week. Also, advance notice that I'm going to be away for part of the next two weeks. I've added a couple of comments for you to address, but as these don't have any impact on the code changes I've pushed a try build as https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b1e40831f576170466ea516cdbf08a03da43e01 to see if the patch causes any regressions.

::: commit-message-79c65:1
(Diff revision 2)
> +Bug 1471920-[mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available

You still need to include the reviewer here. The message should be something like: "Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available; r?davehunt"

::: third_party/python/which/which.py:284
(Diff revision 2)
>      altpath = None
>      exts = None
>      try:
>          optlist, args = getopt.getopt(argv[1:], 'haVvqp:e:',
>              ['help', 'all', 'version', 'verbose', 'quiet', 'path=', 'exts='])
> -    except getopt.GetoptError, msg:
> +    except getopt.GetoptError,msg:

Please restore the space after the comma, so that there are no changes to this file.

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review266100

> You still need to include the reviewer here. The message should be something like: "Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available; r?davehunt"

You actually don't have to when mozreview is used. Once the patch is approved and landed, mozreview will automatically add it.

But please note that mozreview is shutdown by probably early next week, so any remaining code review will have to be continued with phabricator or the old plain splinter-review.

Comment 38

[Preeti[:preeti]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

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

Comment 39

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258602/#review266950

::: third_party/python/which/which.py:284
(Diff revision 3)
>      altpath = None
>      exts = None
>      try:
>          optlist, args = getopt.getopt(argv[1:], 'haVvqp:e:',
>              ['help', 'all', 'version', 'verbose', 'quiet', 'path=', 'exts='])
> -    except getopt.GetoptError, msg:
> +    except getopt.GetoptError,msg:

We still have a change in this file. The try run was successful so once this small change is addressed we can go ahead and land the patch.

Comment 40

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review266952

Comment 41

[Preeti[:preeti]]

> We still have a change in this file. The try run was successful so once this
> small change is addressed we can go ahead and land the patch.

I do not have any space in front of the comma in line 284 of mozilla-unified/third_party/python/which/which.py:284 in my local machine.
But still I am making another pull request. If the problem does not get resolved, please guide me on how to address it.

Comment 42

[Preeti[:preeti]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

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

Comment 43

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #41)
> > We still have a change in this file. The try run was successful so once this
> > small change is addressed we can go ahead and land the patch.
> 
> I do not have any space in front of the comma in line 284 of
> mozilla-unified/third_party/python/which/which.py:284 in my local machine.
> But still I am making another pull request. If the problem does not get
> resolved, please guide me on how to address it.

If you look at https://reviewboard.mozilla.org/r/258602/diff/4#index_header you will see the modification to `third_party/python/which/which.py`. It's not a space in front of the comma, but the removal of the space after the comma, which should be restored. If your local file does not match the one in the review then it's possible you just need to ammend your commit and push again.

Comment 44

[Preeti[:preeti]]

> If you look at https://reviewboard.mozilla.org/r/258602/diff/4#index_header
> you will see the modification to `third_party/python/which/which.py`. It's
> not a space in front of the comma, but the removal of the space after the
> comma, which should be restored. If your local file does not match the one
> in the review then it's possible you just need to ammend your commit and
> push again.

Sorry, I meant that there is no space after the comma in my local file.Could you please  tell me how to ammend my commit?
Last time, I had changed spacing in the commit message, before pushing ,however this did mot work I guess.

Comment 45

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Bug 1471920 - [mozversion] Convert test_filelist to pytest and support Python 3;

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

Comment 46

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #44)
> > If you look at https://reviewboard.mozilla.org/r/258602/diff/4#index_header
> > you will see the modification to `third_party/python/which/which.py`. It's
> > not a space in front of the comma, but the removal of the space after the
> > comma, which should be restored. If your local file does not match the one
> > in the review then it's possible you just need to ammend your commit and
> > push again.
> 
> Sorry, I meant that there is no space after the comma in my local file.Could
> you please  tell me how to ammend my commit?
> Last time, I had changed spacing in the commit message, before pushing
> ,however this did mot work I guess.

You need to add the space, and then you can amend and push like you have for previous changes.

Comment 47

[Preeti[:preeti]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/258604/diff/4-5/

Comment 48

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review267136

Something has gone wrong here, and I suspect it's related to work on bug 1471648 as there are changes to mozlog in the latest commit. If you're wanting to work on multiple bugs concurrently then I would recommend using bookmarks to switch between them. See https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/bookmarks.html for some additional details on using bookmarks.

Comment 49

[Code Review Bot [:reviewbot]]

Comment on attachment 8993996 [details]
Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available;

https://reviewboard.mozilla.org/r/258604/#review267138


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/mozbase/mozlog/mozlog/formatters/xunit.py:13
(Diff revision 5)
>  
>  
>  def format_test_id(test_id):
>      """Take a test id and return something that looks a bit like
>      a class path"""
> -    if not isinstance(test_id, six.string_types):
> +    if type(test_id) not in types.StringTypes:

Error: Undefined name 'types' [flake8: F821]

Comment 50

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #48)
> Comment on attachment 8993996 [details]
> Bug 1471920 - [mozversion] Enable some tests on Python 3 by using
> shutil.which in mozbuild where available;
> 
> https://reviewboard.mozilla.org/r/258604/#review267136
> 
> Something has gone wrong here, and I suspect it's related to work on bug
> 1471648 as there are changes to mozlog in the latest commit. If you're
> wanting to work on multiple bugs concurrently then I would recommend using
> bookmarks to switch between them. See
> https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> bookmarks.html for some additional details on using bookmarks.

Do I need to make a bookmark  while I push some changes next time or do I make one now?
How do I make a bookmark now?

Comment 51

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #50)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #48)
> > Comment on attachment 8993996 [details]
> > Bug 1471920 - [mozversion] Enable some tests on Python 3 by using
> > shutil.which in mozbuild where available;
> > 
> > https://reviewboard.mozilla.org/r/258604/#review267136
> > 
> > Something has gone wrong here, and I suspect it's related to work on bug
> > 1471648 as there are changes to mozlog in the latest commit. If you're
> > wanting to work on multiple bugs concurrently then I would recommend using
> > bookmarks to switch between them. See
> > https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> > bookmarks.html for some additional details on using bookmarks.
> 
> Do I need to make a bookmark  while I push some changes next time or do I
> make one now?
> How do I make a bookmark now?

You should be able to make a bookmark at any time. I would suggest having a bookmark against your latest commit for this bug, and name it after the bug, such as "bug1471920". You can then update back to the "central" bookmark, add a new commit for another bug, and then create another bookmark for that work. This will allow you to switch back and forth between bookmarks, and to push for review from whichever is active.

Comment 52

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #51)
> (In reply to Preeti from comment #50)
> > (In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #48)
> > > Comment on attachment 8993996 [details]
> > > Bug 1471920 - [mozversion] Enable some tests on Python 3 by using
> > > shutil.which in mozbuild where available;
> > > 
> > > https://reviewboard.mozilla.org/r/258604/#review267136
> > > 
> > > Something has gone wrong here, and I suspect it's related to work on bug
> > > 1471648 as there are changes to mozlog in the latest commit. If you're
> > > wanting to work on multiple bugs concurrently then I would recommend using
> > > bookmarks to switch between them. See
> > > https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> > > bookmarks.html for some additional details on using bookmarks.
> > 
> > Do I need to make a bookmark  while I push some changes next time or do I
> > make one now?
> > How do I make a bookmark now?
> 
> You should be able to make a bookmark at any time. I would suggest having a
> bookmark against your latest commit for this bug, and name it after the bug,
> such as "bug1471920". You can then update back to the "central" bookmark,
> add a new commit for another bug, and then create another bookmark for that
> work. This will allow you to switch back and forth between bookmarks, and to
> push for review from whichever is active.

I am getting the following output. Please do tell what to  do next:

preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ code which.py
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg bookmark bug1471920
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
M ../../../testing/mozbase/mozlog/tests/manifest.ini
M which.py
? ../../../.hgrc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg bookmark bug1471920
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
M ../../../testing/mozbase/mozlog/tests/manifest.ini
M which.py
? ../../../.hgrc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push -B bug1471920
pushing to https://hg.mozilla.org/mozilla-unified
searching for changes
no changes found
abort: authorization failed
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg bookmark bug1471920
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push -B bug1471920
pushing to https://hg.mozilla.org/mozilla-unified
searching for changes
no changes found
abort: authorization failed
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
M ../../../testing/mozbase/mozlog/tests/manifest.ini
M which.py
? ../../../.hgrc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg bookmarks
 * bug1471920                478560:a05794e50453
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg add which.py
which.py already tracked!
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg commit -m "Space removed in line 284 of which.py"
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push -r bug1471920
pushing to https://hg.mozilla.org/mozilla-unified
searching for changes
remote has heads on branch 'default' that are not known locally: 9b8da5415bd6 a55de03eb91d dfa6bf6bc3b8
abort: push creates new remote head e13efb85abb4 with bookmark 'bug1471920'!
(pull and merge or see 'hg help push' for details about pushing new heads)
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push -f -r bug1471920
pushing to https://hg.mozilla.org/mozilla-unified
searching for changes
abort: authorization failed                                                     
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push -B bug1471920
pushing to https://hg.mozilla.org/mozilla-unified
searching for changes
remote has heads on branch 'default' that are not known locally: 9b8da5415bd6 a55de03eb91d dfa6bf6bc3b8
abort: authorization failed                                                     
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
? ../../../.hgrc

Comment 53

[Dave Hunt [:davehunt] [he/him] ⌚BST]

You don't need to push your bookmarks, you can just use them locally for organising your work on different patches. Here's how I use them:

> // start on the central bookmark
> $ hg up central

> // pull in any upstream changes
> $ hg pull -u

> // start working on a patch
> // add any new files
> $ hg add my/new/files

> // commit my changes
> $ hg commit -m 'Bug XYZ - My commit message; r?reviewer'

> // create a bookmark
> $ hg bookmark bugXYZ

Now when I need to work on a separate patch, I repeat the above but replace would use the appropriate bug number in the bookmark name and commit message. Here are a few additional commands I use:

> // see work in progress, with indication of active bookmark
> $ hg wip

> // push active commit(s) for review
> $ hg push review

When pushing to review, the current context will be taken into consideration. So if you're on a bookmark, the latest commit and parents that deviate from central will be pushed. I hope that helps!

Comment 54

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #53)
> You don't need to push your bookmarks, you can just use them locally for
> organising your work on different patches. Here's how I use them:
> 
> > // start on the central bookmark
> > $ hg up central
> 
> > // pull in any upstream changes
> > $ hg pull -u
> 
> > // start working on a patch
> > // add any new files
> > $ hg add my/new/files
> 
> > // commit my changes
> > $ hg commit -m 'Bug XYZ - My commit message; r?reviewer'
> 
> > // create a bookmark
> > $ hg bookmark bugXYZ
> 
> Now when I need to work on a separate patch, I repeat the above but replace
> would use the appropriate bug number in the bookmark name and commit
> message. Here are a few additional commands I use:
> 
> > // see work in progress, with indication of active bookmark
> > $ hg wip
> 
> > // push active commit(s) for review
> > $ hg push review
> 
> When pushing to review, the current context will be taken into
> consideration. So if you're on a bookmark, the latest commit and parents
> that deviate from central will be pushed. I hope that helps!

Thank you! Your  explanation has helped me a lot. 
However, I get the following output:


preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg up central
abort: conflicting changes
(commit or update --clean to discard changes)
preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg pull -u
pulling from https://hg.mozilla.org/mozilla-unified
searching for changes
adding changesets
adding manifests                                                                
adding file changes
added 756 changesets with 4083 changes to 2576 files (+1 heads)                 
updated firefox tree tag beta (+26 commits)
updated firefox tree tag central (+480 commits)
updated firefox tree tag esr52 (+6 commits)
updated firefox tree tag inbound (+537 commits)
new changesets ce5febb8f863:3b82435477e6
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
preeti@preeti-Inspiron-5577:~/mozilla-unified$ cd third_party/python/which/
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg add  which.py
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
A which.py
? LICENSE.txt
? MANIFEST.in
? Makefile.win
? PKG-INFO
? README.txt
? TODO.txt
? build.py
? launcher.cpp
? logo.jpg
? setup.py
? test/test_which.py
? test/testsupport.py
? which.pyc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg commit -m 'Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available; r?davehunt'
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg bookmark bug1471920
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg wip
@  0:d8978bb6a6bb preetimukherjee98 tip bug1471920 Bug 1471920 - [mozversion] Enable some tests on Python 3 by using shutil.which in mozbuild where available; r?davehunt
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg push review
pushing to https://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
abort: no review repository found

Comment 55

[Dave Hunt [:davehunt] [he/him] ⌚BST]

It seems you hit an issue pushing to mozreview. How did you previously push to review? Was it using `hg push review` or by another means. If another means, please try doing that again. If not, it may be that we are no longer accepting patches to mozreview.

It is unfortunately that you started working on this patch at a time that we're transitioning to a new review tool. I would suggest taking some time to read through the documentation at http://moz-conduit.readthedocs.io/en/latest/ and perhaps attempt to submit your review to Phabricator.

I will be away from tomorrow for a week, so there may be a delay in further responses for a while. I've asked :raphael to help out where he can, but please request needinfo on any comments that you'd like a response on. You can do this by checking the box labelled "Need more information from" and type :raphael in the input box, or :davehunt if you're okay with there being some delay.

Thanks for your perseveration!

Comment 56

[Preeti[:preeti]]

Need more information from:raphael

I have successfully installed Phabricator , but am still unable to push the change required. Could you please tell me how to push the which.py file? I have pasted a few outputs below:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ arc install-certificate
 CONNECT  Connecting to "https://phabricator.services.mozilla.com/api/"...
LOGIN TO PHABRICATOR
Open this page in your browser and login to Phabricator if necessary:

https://phabricator.services.mozilla.com/conduit/login/

Then paste the API Token on that page below.

    Paste API Token from that page: ----------------
Writing ~/.arcrc...
 SUCCESS!  API Token installed.
preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg status
! third_party/python/which/LICENSE.txt
! third_party/python/which/MANIFEST.in
! third_party/python/which/Makefile.win
! third_party/python/which/PKG-INFO
! third_party/python/which/README.txt
! third_party/python/which/TODO.txt
! third_party/python/which/build.py
! third_party/python/which/launcher.cpp
! third_party/python/which/logo.jpg
! third_party/python/which/setup.py
! third_party/python/which/test/test_which.py
! third_party/python/which/test/testsupport.py
! third_party/python/which/which.py
? .hgrc
? arcanist/bin/.arcconfig
preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg add which.py
which.py: No such file or directory
preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg add third_party/python/which/which.py
abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'
preeti@preeti-Inspiron-5577:~/mozilla-unified$ cd third_party/python/which
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg commit -m "Bug 1471920 - Removal of space in third_party/python/which; r?reviewer
> hg commit -m "Bug 1471920 - Removal of space in third_party/python/which; r?reviewer^C
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg commit -m "Bug 1471920 - Removal of space in line 284 of third_party/python/which/which.py; r?davehunt"
nothing changed
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
? LICENSE.txt
? MANIFEST.in
? Makefile.win
? PKG-INFO
? README.txt
? TODO.txt
? build.py
? launcher.cpp
? logo.jpg
? setup.py
? test/test_which.py
? test/testsupport.py
? which.pyc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ code which.py

preeti@preeti-Inspiron-5577:~/mozilla-unified$ arc diff
Usage Exception: You have missing files in this working copy. Revert or formally remove them (with `svn rm`) before proceeding.

  Working copy: /home/preeti/mozilla-unified/

  Missing files in working copy:
    third_party/python/which/LICENSE.txt
    third_party/python/which/MANIFEST.in
    third_party/python/which/Makefile.win
    third_party/python/which/PKG-INFO
    third_party/python/which/README.txt
    third_party/python/which/TODO.txt
    third_party/python/which/build.py
    third_party/python/which/launcher.cpp
    third_party/python/which/logo.jpg
    third_party/python/which/setup.py
    third_party/python/which/test/test_which.py
    third_party/python/which/test/testsupport.py
    third_party/python/which/which.py

preeti@preeti-Inspiron-5577:~/mozilla-unified$ cd third_party/python/which/
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ arc diffUsage Exception: This command requires arc to connect to a Phabricator install, but no
Phabricator installation is configured. To configure a Phabricator URI:

  - set a default location with `arc set-config default <uri>`; or
  - specify `--conduit-uri=uri` explicitly; or
  - run `arc` in a working copy with an '.arcconfig'.

Comment 57

[Andrew Halberstadt [:ahal]]

Comment on attachment 8995712 [details]
Bug 1471920 - [mozversion] Convert test_filelist to pytest and support Python 3;

https://reviewboard.mozilla.org/r/260096/#review268726

Lgtm!

Comment 58

[Raphael Aurich [:raphael] UTC+02:00]

Comment on attachment 8995712 [details]
Bug 1471920 - [mozversion] Convert test_filelist to pytest and support Python 3;

https://reviewboard.mozilla.org/r/260096/#review268732

Two comments on comprehensibility.

::: testing/mozbase/mozhttpd/tests/filelisting.py:21
(Diff revision 1)
>  
>  here = os.path.dirname(os.path.abspath(__file__))
>  
>  
> -class FileListingTest(unittest.TestCase):
> -
> [e-mail](name='dirlist', params=['', '?foo=bar&fleem=&foo=fleem'])
> +def fixture_dirlist(request):

Can you please add ids for the parameters?

Something like:

```python
@pytest.fixture(
    name='dirlist',
    params=[
        '',
        '?foo=bar&fleem=&foo=fleem',
    ],
    ids=[
        'no_query',
        'query',
    ],
)
def fixture_dirlist(request):
    pass
```

::: testing/mozbase/mozhttpd/tests/filelisting.py:27
(Diff revision 1)
> -        httpd = mozhttpd.MozHttpd(port=0, docroot=here)
> +    httpd = mozhttpd.MozHttpd(port=0, docroot=here)
> -        httpd.start(block=False)
> +    httpd.start(block=False)
> -        f = urllib2.urlopen("http://%s:%s/%s" % ('127.0.0.1', httpd.httpd.server_port, path))
> -        for line in f.readlines():
> -            webline = re.sub('\<[a-zA-Z0-9\-\_\.\=\"\'\/\\\%\!\@\#\$\^\&\*\(\) ]*\>',
> -                             '', line.strip('\n')).strip('/').strip().strip('@')
> +    f = urlopen('http://%s:%s/%s' % ('127.0.0.1', httpd.httpd.server_port, request.param))
> +    pattern = b'<a.*>(.*)<\/a>'
> +    dirlist = filter(lambda x: re.search(pattern, x), f.readlines())
> +    yield list(map(lambda x: re.search(pattern, x).group(1).decode(), dirlist))

Most likely a matter of personal preference, but I find this slightly easier to comprehend:

```python
yield [
    re.search(pattern, line).group(1).decode()
    for line in lines if re.search(pattern, line)
]
```

Comment 59

[Preeti[:preeti]]

I have done the modifications and am getting the following output:

preeti@preeti-Inspiron-5577:~/mozilla-unified/testing/mozbase/mozhttpd/tests$ hg add filelisting.py 
filelisting.py already tracked!
preeti@preeti-Inspiron-5577:~/mozilla-unified/testing/mozbase/mozhttpd/tests$ hg commit -m "Bug 1471920 - Modifications for comprehensibility; r?raphael,davehunt"
abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'
preeti@preeti-Inspiron-5577:~/mozilla-unified/testing/mozbase/mozhttpd/tests$ hg push review
pushing to https://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
redirecting push to https://reviewboard-hg.mozilla.org/gecko
(adding commit id to 1 changesets)
abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'

Comment 60

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Sorry for the confusion, it appears I pushed a patch for review on bug 1428709 with the wrong bug number, so the review comments have appeared here. I'm not entirely sure how to fix that, so I'll tag the comments in Bugzilla so they're hidden and close the review. I'll submit a new patch later with the correct bug number and the comments addressed.

Comment 61

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Let's ignore the Phabricator issues for now, as there appear to be other issues with your local repository. The following message is concerning:

> abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'

I'm not sure why it's suggesting there's a nested repo in that location. I would suggest reverting any changes to third_party/python/which as fixing this bug does not require changes to these files.

Comment 62

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #61)
> Let's ignore the Phabricator issues for now, as there appear to be other
> issues with your local repository. The following message is concerning:
> 
> > abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'
> 
> I'm not sure why it's suggesting there's a nested repo in that location. I
> would suggest reverting any changes to third_party/python/which as fixing
> this bug does not require changes to these files.

Sorry for the  delay. So do I discard all the changes I made for which.py? I have not made any other change in third_party/python/which directory.

Comment 63

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #62)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #61)
> > Let's ignore the Phabricator issues for now, as there appear to be other
> > issues with your local repository. The following message is concerning:
> > 
> > > abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'
> > 
> > I'm not sure why it's suggesting there's a nested repo in that location. I
> > would suggest reverting any changes to third_party/python/which as fixing
> > this bug does not require changes to these files.
> 
> Sorry for the  delay. So do I discard all the changes I made for which.py? I
> have not made any other change in third_party/python/which directory.

That's correct, you should not need to make any changes to the vendored third party packages.

Comment 64

[Preeti[:preeti]]

> That's correct, you should not need to make any changes to the vendored
> third party packages.

I have restored the which.py file to it's original state in my local machine. Do I need to push the changes?
Further, please guide me on how to proceed now.

Comment 65

[Dave Hunt [:davehunt] [he/him] ⌚BST]

As your previous commit included changes to this file you'll need to amend it to restore the original content. You should be able to check the changes using `hg diff` and then amend the commit using `hg commit --amend`. Once this is done, `hg push review` should update your review request.

It does seem though that your latest push to mozreview for this bug contains a lot of unrelated changes to mozlog. As the MozReview tool is due for decommision in less than a week it may make sense for you to submit a new review as a patch attachment to this bug or to our new review tool, Phabricator.

Details for setting up Phabricator can be found here: https://moz-conduit.readthedocs.io/en/latest/

Sorry for the unfortunate timing of our change of review tool.

Comment 66

[Preeti[:preeti]]

I had previously installed phabrcator (please  see comment #56). But I am getting the following output:

preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg status
M which.py
? LICENSE.txt
? MANIFEST.in
? Makefile.win
? PKG-INFO
? README.txt
? TODO.txt
? build.py
? launcher.cpp
? logo.jpg
? setup.py
? test/test_which.py
? test/testsupport.py
? which.pyc
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ hg commit --amend
saved backup bundle to /home/preeti/mozilla-unified/third_party/python/which/.hg/strip-backup/d8978bb6a6bb-80d7c7d2-amend.hg
preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ arc diffUsage Exception: This command requires arc to connect to a Phabricator install, but no
Phabricator installation is configured. To configure a Phabricator URI:

  - set a default location with `arc set-config default <uri>`; or
  - specify `--conduit-uri=uri` explicitly; or
  - run `arc` in a working copy with an '.arcconfig'.

preeti@preeti-Inspiron-5577:~/mozilla-unified/third_party/python/which$ arc install-certificate
Usage Exception: Specify an explicit URI or run this command from within a project which is configured with a .arcconfig.


I want to contribute to open source projects and want also do internships on the same under Mozilla. Since I have just started my journey in open source, I am always happy to get an opportunity learn new things!Thank You!

Comment 67

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Can you try running `arc` from the `~/mozilla-unified` directory? This contains the `.arcconfig` that `arc` is looking for.

Comment 68

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #67)
> Can you try running `arc` from the `~/mozilla-unified` directory? This
> contains the `.arcconfig` that `arc` is looking for.

I am getting the following output:

preeti@preeti-Inspiron-5577:~$ cd mozilla-unified
preeti@preeti-Inspiron-5577:~/mozilla-unified$ arc
Usage Exception: No command provided. Try `arc help`.
preeti@preeti-Inspiron-5577:~/mozilla-unified$ arc diff
Usage Exception: You have missing files in this working copy. Revert or formally remove them (with `svn rm`) before proceeding.

  Working copy: /home/preeti/mozilla-unified/

  Missing files in working copy:
    third_party/python/which/LICENSE.txt
    third_party/python/which/MANIFEST.in
    third_party/python/which/Makefile.win
    third_party/python/which/PKG-INFO
    third_party/python/which/README.txt
    third_party/python/which/TODO.txt
    third_party/python/which/build.py
    third_party/python/which/launcher.cpp
    third_party/python/which/logo.jpg
    third_party/python/which/setup.py
    third_party/python/which/test/test_which.py
    third_party/python/which/test/testsupport.py
    third_party/python/which/which.py

Comment 69

[Dave Hunt [:davehunt] [he/him] ⌚BST]

hmm.. it would seem that `third_party/python/which` has been removed from your workspace. Can you try restoring it using `hg revert -r central third_party/python/which`

Comment 70

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #69)
> hmm.. it would seem that `third_party/python/which` has been removed from
> your workspace. Can you try restoring it using `hg revert -r central
> third_party/python/which`

I am getting the following output:

preeti@preeti-Inspiron-5577:~/mozilla-unified$ hg revert -r central third_party/python/which
abort: path 'third_party/python/which/which.py' is inside nested repo 'third_party/python/which'

Comment 71

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Do you have a repository within `third_party/python/which`? What does `ls -al third_party/python/which` return?

Comment 72

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #71)
> Do you have a repository within `third_party/python/which`? 
Yes, I do have a repository there in third_party/python/which, because I had once tried to push only the which.py file by creating a repository there.

What does `ls
> -al third_party/python/which` return?
preeti@preeti-Inspiron-5577:~/mozilla-unified$ ls -al third_party/python/which
total 108
drwxr-xr-x  4 preeti preeti  4096 Aug 15 23:18 .
drwxr-xr-x 45 preeti preeti  4096 Jul 16 20:13 ..
-rw-r--r--  1 preeti preeti 13282 Jul 16 00:32 build.py
drwxr-xr-x  6 preeti preeti  4096 Aug 15 23:17 .hg
-rw-r--r--  1 preeti preeti 11713 Jul 16 00:32 launcher.cpp
-rw-r--r--  1 preeti preeti  1067 Jul 16 00:32 LICENSE.txt
-rw-r--r--  1 preeti preeti  3635 Jul 16 00:32 logo.jpg
-rw-r--r--  1 preeti preeti   650 Jul 16 00:32 Makefile.win
-rw-r--r--  1 preeti preeti   104 Jul 16 00:32 MANIFEST.in
-rw-r--r--  1 preeti preeti   713 Jul 16 00:32 PKG-INFO
-rw-r--r--  1 preeti preeti  7465 Jul 16 00:32 README.txt
-rw-r--r--  1 preeti preeti  1957 Jul 16 00:32 setup.py
drwxr-xr-x  2 preeti preeti  4096 Jul 16 00:32 test
-rw-r--r--  1 preeti preeti  4943 Jul 16 00:32 TODO.txt
-rw-r--r--  1 preeti preeti 12168 Aug 15 23:12 which.py
-rw-r--r--  1 preeti preeti 11371 Jul 28 20:56 which.pyc

Comment 73

[Dave Hunt [:davehunt] [he/him] ⌚BST]

It does seem that you have a repository in `third_party/python/which`:

> drwxr-xr-x  6 preeti preeti  4096 Aug 15 23:17 .hg

I'm not quite sure how that got there... You could execute `hg paths` from `third_party/python/which` to see where that repository is pointing to, but it shouldn't matter as we should be able to safely delete it...

Try running `rm -rf third_party/python/which/.hg` and then try pushing for review again.

Comment 74

[Preeti[:preeti]]

Attachment: Space removed in line 284 of which.py

Bug 1471920 - Discard all changes in third_party/python/which; r?davehunt

Comment 75

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #73)
> It does seem that you have a repository in `third_party/python/which`:
> 
> > drwxr-xr-x  6 preeti preeti  4096 Aug 15 23:17 .hg
> 
> I'm not quite sure how that got there... You could execute `hg paths` from
> `third_party/python/which` to see where that repository is pointing to, but
> it shouldn't matter as we should be able to safely delete it...
> 
> Try running `rm -rf third_party/python/which/.hg` and then try pushing for
> review again.

I have pushed the code in phabricator for review! Please do let me know if I need to make any further chenges.

Comment 76

[Preeti[:preeti]]

(In reply to Preeti from comment #75)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #73)
> > It does seem that you have a repository in `third_party/python/which`:
> > 
> > > drwxr-xr-x  6 preeti preeti  4096 Aug 15 23:17 .hg
> > 
> > I'm not quite sure how that got there... You could execute `hg paths` from
> > `third_party/python/which` to see where that repository is pointing to, but
> > it shouldn't matter as we should be able to safely delete it...
> > 
> > Try running `rm -rf third_party/python/which/.hg` and then try pushing for
> > review again.
> 
> I have pushed the code in phabricator for review! Please do let me know if I
> need to make any further chenges.
As per your comments in Phabricator, I have the following doubts:
Could you please elaborate on how to update to the central bookmark?
So, do I add a white space in third_party/python/which/which.py and then push it again for review?

Comment 77

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti from comment #76)
> (In reply to Preeti from comment #75)
> > (In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #73)
> > > It does seem that you have a repository in `third_party/python/which`:
> > > 
> > > > drwxr-xr-x  6 preeti preeti  4096 Aug 15 23:17 .hg
> > > 
> > > I'm not quite sure how that got there... You could execute `hg paths` from
> > > `third_party/python/which` to see where that repository is pointing to, but
> > > it shouldn't matter as we should be able to safely delete it...
> > > 
> > > Try running `rm -rf third_party/python/which/.hg` and then try pushing for
> > > review again.
> > 
> > I have pushed the code in phabricator for review! Please do let me know if I
> > need to make any further chenges.
> As per your comments in Phabricator, I have the following doubts:
> Could you please elaborate on how to update to the central bookmark?
> So, do I add a white space in third_party/python/which/which.py and then
> push it again for review?

Let's not duplicate the conversation here. I have responded in Phabricator.

Comment 78

[Preeti[:preeti]]

Attachment: Bug 1471920 - [mozbuild] Use shutil.which in Python 3 instead of vendored third-party package

Restoration of changes of files in testing/mozbase/mozversion

Comment 79

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 9003691 [details]
Bug 1471920 - [mozbuild] Use shutil.which in Python 3 instead of vendored third-party package

Dave Hunt [:davehunt] ⌚️UTC+1 has approved the revision.

Comment 80

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4efdc36c40f7
[mozbuild] Use shutil.which in Python 3 instead of vendored third-party package r=davehunt

Comment 81

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The commit that's been pushed fixes most of the tests. Let's leave the bug open to take care of the remaining items.

Comment 82

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/4efdc36c40f7

Comment 83

[Dave Hunt [:davehunt] [he/him] ⌚BST]

:preeti your first commit has landed, and we've enabled 10/12 tests against Python 3! Are you still interested in working on the remaining items? When I looked into it I believe it was related to changes in ZipFile where we are able to directly read the contents of a file in Python 2, but not in Python 3. It might be that we need to unzip the paths to a temporary location before reading them, unless we can find a way to do it all in memory like we do currently.

Comment 84

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #83)
> :preeti your first commit has landed, and we've enabled 10/12 tests against
> Python 3! Are you still interested in working on the remaining items? When I
> looked into it I believe it was related to changes in ZipFile where we are
> able to directly read the contents of a file in Python 2, but not in Python
> 3. It might be that we need to unzip the paths to a temporary location
> before reading them, unless we can find a way to do it all in memory like we
> do currently.

Sure Sir! But I do not know how to unzip paths in python, so could you please suggest some resources on the same?

Comment 85

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The failure is cause by this line: https://searchfox.org/mozilla-central/source/testing/mozbase/mozversion/mozversion/mozversion.py#66-67, which is calling `archive.open`. As `archive` is a `ZipFile` instance, you can read the documentation for the Python 3 implementation at https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.open. I would check the return types between Python 2.7 and 3.

The returned object is being passed directly to ConfigParser.readfp, which is documented here: https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.readfp. There's a note about a deprecation and change of behaviour in Python 3.2, which you may want to investigate.

Comment 86

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #85)
> The failure is cause by this line:
> https://searchfox.org/mozilla-central/source/testing/mozbase/mozversion/
> mozversion/mozversion.py#66-67, which is calling `archive.open`. As
> `archive` is a `ZipFile` instance, you can read the documentation for the
> Python 3 implementation at
> https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.open. I would
> check the return types between Python 2.7 and 3.
> 
> The returned object is being passed directly to ConfigParser.readfp, which
> is documented here:
> https://docs.python.org/3/library/configparser.html#configparser.
> ConfigParser.readfp. There's a note about a deprecation and change of
> behaviour in Python 3.2, which you may want to investigate.

Should we consider placing an if else block in line https://searchfox.org/mozilla-central/source/testing/mozbase/mozversion/mozversion/mozversion.py#66-67 where we check the python version is 2.7 then it executes the if part or the existing code, otherwise it executes the else part or the part where we keep in mind the syntax for python 3? However could you also suggest methods of testing python version at run-time?

Comment 87

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti[:preeti] from comment #86)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #85)
> > The failure is cause by this line:
> > https://searchfox.org/mozilla-central/source/testing/mozbase/mozversion/
> > mozversion/mozversion.py#66-67, which is calling `archive.open`. As
> > `archive` is a `ZipFile` instance, you can read the documentation for the
> > Python 3 implementation at
> > https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.open. I would
> > check the return types between Python 2.7 and 3.
> > 
> > The returned object is being passed directly to ConfigParser.readfp, which
> > is documented here:
> > https://docs.python.org/3/library/configparser.html#configparser.
> > ConfigParser.readfp. There's a note about a deprecation and change of
> > behaviour in Python 3.2, which you may want to investigate.
> 
> Should we consider placing an if else block in line
> https://searchfox.org/mozilla-central/source/testing/mozbase/mozversion/
> mozversion/mozversion.py#66-67 where we check the python version is 2.7 then
> it executes the if part or the existing code, otherwise it executes the else
> part or the part where we keep in mind the syntax for python 3? However
> could you also suggest methods of testing python version at run-time?

I think using a if/else should be a last resort for Python 3 compatibility. Let's explore some other options first. Looking into this a bit more, the issue appears to be that Python 3 is returning a bytes-like file object from `ZipFile.open`, and `configparser.readfp` is expecting a text file. Perhaps look into TextIOWrapper (https://docs.python.org/3.7/library/io.html#io.TextIOWrapper) to see if we can decode the binary object before passing it to `_parse_ini_file`.

Comment 88

[Preeti[:preeti]]

> I think using a if/else should be a last resort for Python 3 compatibility.
> Let's explore some other options first. Looking into this a bit more, the
> issue appears to be that Python 3 is returning a bytes-like file object from
> `ZipFile.open`, and `configparser.readfp` is expecting a text file. Perhaps
> look into TextIOWrapper
> (https://docs.python.org/3.7/library/io.html#io.TextIOWrapper) to see if we
> can decode the binary object before passing it to `_parse_ini_file`.

Could you please elaborate on "decode the binary object"?

Comment 89

[Dave Hunt [:davehunt] [he/him] ⌚BST]

In Python 3, `ZipFile.open` method is returning a binary file-like object, which means it's an object that represents the bytes in the file specified from the archive. This makes a lot of sense, because the file could be an image, an executable, or indeed anything! We happen to know that the file we're opening is a text file, and we want to pass it to another method that is expecting a text file, so we should be able to safely decode the binary file-like object to a text file-like object. This isn't necessary in Python 2 because `ZipFile.open` already returns a text file-like object.

You might find the following links useful:
https://snarky.ca/why-python-3-exists/
https://portingguide.readthedocs.io/en/latest/strings.html#file-i-o

Comment 90

[Preeti[:preeti]]

Can this be a possible solution?:(All the changes I am mentioning is for file mozilla-central/testing/mozbase/mozversion/mozversion/mozversion.py)
1. Add  "from io import open" in the beginning.
2. Before line #66 add:
    filename=class io.TextIOWrapper(buffer, encoding=None, errors=None, newline=None, line_buffering=False, write_through=False)
    with open('filename', 'rb') as f:
        f.read()  # bytes
Please do correct me if I am thinking in the wrong direction.

Comment 91

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti[:preeti] from comment #90)
> Can this be a possible solution?:(All the changes I am mentioning is for
> file mozilla-central/testing/mozbase/mozversion/mozversion/mozversion.py)
> 1. Add  "from io import open" in the beginning.
> 2. Before line #66 add:
>     filename=class io.TextIOWrapper(buffer, encoding=None, errors=None,
> newline=None, line_buffering=False, write_through=False)
>     with open('filename', 'rb') as f:
>         f.read()  # bytes
> Please do correct me if I am thinking in the wrong direction.

It sounds like you have the right idea, although the code in your comment wouldn't work as it is. See if you can get a patch working locally and push it for review, where we can discuss the changes.

Comment 92

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #91)
> (In reply to Preeti[:preeti] from comment #90)
> > Can this be a possible solution?:(All the changes I am mentioning is for
> > file mozilla-central/testing/mozbase/mozversion/mozversion/mozversion.py)
> > 1. Add  "from io import open" in the beginning.
> > 2. Before line #66 add:
> >     filename=class io.TextIOWrapper(buffer, encoding=None, errors=None,
> > newline=None, line_buffering=False, write_through=False)
> >     with open('filename', 'rb') as f:
> >         f.read()  # bytes
> > Please do correct me if I am thinking in the wrong direction.
> 
> It sounds like you have the right idea, although the code in your comment
> wouldn't work as it is. See if you can get a patch working locally and push
> it for review, where we can discuss the changes.

I am getting the following output:
preeti@preeti-Inspiron-5577:~/mozilla-unified$ ./mach python-test --python=3.5 testing/mozbase/mozversion
Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
 0:11.23 /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:11.25 Traceback (most recent call last):
 0:11.25   File "/home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py", line 17, in <module>
 0:11.25     from mozversion import errors, get_version
 0:11.25   File "/home/preeti/mozilla-unified/testing/mozbase/mozversion/mozversion/__init__.py", line 12, in <module>
 0:11.25     from .mozversion import cli, get_version
 0:11.25   File "/home/preeti/mozilla-unified/testing/mozbase/mozversion/mozversion/mozversion.py", line 66
 0:11.25     filename=class io.TextIOWrapper(buffer, encoding=None, errors=None, newline=None, line_buffering=False, write_through=False)
 0:11.25                  ^
 0:11.25 SyntaxError: invalid syntax
 0:11.25 TEST-UNEXPECTED-FAIL | No test output (missing mozunit.main() call?): /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:11.25 Setting retcode to 1 from /home/preeti/mozilla-unified/testing/mozbase/mozversion/tests/test_binary.py
 0:11.25 Return code from mach python-test: 1


However while pushing the code, I am getting the following error:
preeti@preeti-Inspiron-5577:~/mozilla-unified$ arc diff
You have untracked files in this working copy.

  Working copy: /home/preeti/mozilla-unified/

  Untracked changes in working copy:
  (To ignore these changes, add them to ".hgignore".)
    .hgrc
    PHABTEST
    arcanist/bin/.arcconfig

    Ignore these untracked files and continue? [y/N] y

Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Phabricator does not support staging areas for this repository.
 Exception 
ERR_CLOSED: This revision has already been closed.
(Run with `--trace` for a full exception trace.)

Comment 93

[Andrew Halberstadt [:ahal]]

It looks like your changes introduced a SyntaxError in mozversion.py, you'll need to figure out how to fix it :)

Comment 94

[Preeti[:preeti]]

Can someone please guide me on how to address this syntactic error?

Comment 95

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti[:preeti] from comment #94)
> Can someone please guide me on how to address this syntactic error?

The reserved word `class` is used for definining a class in Python. It looks like you've pasted the class signature instead of using the class from the imported module to create an object. Perhaps you copied from the class documentation, rather than an example using the class? I would recommend Dive into Python (http://www.diveintopython3.net/) as a good introduction to programming with Python, where classes are covered in chapter 7. I would also recommend using an editor that can highlight these syntax errors, and perhaps make writing Python easier by making autocomplete suggestions.

Comment 96

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Are you still interested in working on this Preeti? Do you need anything more from me to help you to proceed?

Comment 97

[Preeti[:preeti]]

(In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #96)
> Are you still interested in working on this Preeti? Do you need anything
> more from me to help you to proceed?

Yes, I would still like to work on this bug.
As per your advice, I have used VS Code, where I am getting a pylint E0001 error in mozversion.py . By [1] this error message stands for " (syntax error raised for a module; message varies)".I was looking for similar files but have not got any till now.What do I do now? Please let me know.  


[1]http://pylint-messages.wikidot.com/all-codes

Comment 98

[Preeti[:preeti]]

Please de-assign me from this bug as I am still unable to find an appropriate solution for this bug. Thanks in advance!

Comment 99

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti[:preeti] from comment #97)
> (In reply to Dave Hunt [:davehunt] ⌚️UTC+1 from comment #96)
> > Are you still interested in working on this Preeti? Do you need anything
> > more from me to help you to proceed?
> 
> Yes, I would still like to work on this bug.
> As per your advice, I have used VS Code, where I am getting a pylint E0001
> error in mozversion.py . By [1] this error message stands for " (syntax
> error raised for a module; message varies)".I was looking for similar files
> but have not got any till now.What do I do now? Please let me know.  
> 
> 
> [1]http://pylint-messages.wikidot.com/all-codes

Hmm.. it sounds like this might be a general code to cover one or more specific codes. Is this before or after your changes?

(In reply to Preeti[:preeti] from comment #98)
> Please de-assign me from this bug as I am still unable to find an
> appropriate solution for this bug. Thanks in advance!

I hope I can convince you to finish off this bug. I believe what we need to do is wrap the result of `archive.open(filename)` so that it can be treated as text file-like object instead of not knowing if it's text (python 2) or a binary file-like object (python 3). At the moment we're assuming it's text and passing it directly into `self._parse_ini_file`. Instead, we could create a new intermediate object, using `io.TextIOWrapper`. Something like: `fp = io.TextIOWrapper(archive.open(filename))`. Then, we can pass this new `fp` object to `self._parse_ini_file`. There are two places where we call `archive.open` so you'll need to repeat this in both places.

Let me know if you'd like some additional help, or if you'd still like this to be unassigned from you. Either way, your contributions so far are very much appreciated!

Comment 100

[Preeti[:preeti]]

 Is this before or after your changes?

This was after I made the changes.

> 
> I hope I can convince you to finish off this bug. I believe what we need to
> do is wrap the result of `archive.open(filename)` so that it can be treated
> as text file-like object instead of not knowing if it's text (python 2) or a
> binary file-like object (python 3). At the moment we're assuming it's text
> and passing it directly into `self._parse_ini_file`. Instead, we could
> create a new intermediate object, using `io.TextIOWrapper`. Something like:
> `fp = io.TextIOWrapper(archive.open(filename))`. Then, we can pass this new
> `fp` object to `self._parse_ini_file`. There are two places where we call
> `archive.open` so you'll need to repeat this in both places.
>

I have made the suggested changes in mozversion.py but I cannot find the occurrence of archive.open , could you please help me?
I tried ```grep -roh archive.open . | wc -w``` in the mozilla-unified folder, but it did not return any result.

Comment 101

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Preeti[:preeti] from comment #100)
>  Is this before or after your changes?
> 
> This was after I made the changes.
> 
> > 
> > I hope I can convince you to finish off this bug. I believe what we need to
> > do is wrap the result of `archive.open(filename)` so that it can be treated
> > as text file-like object instead of not knowing if it's text (python 2) or a
> > binary file-like object (python 3). At the moment we're assuming it's text
> > and passing it directly into `self._parse_ini_file`. Instead, we could
> > create a new intermediate object, using `io.TextIOWrapper`. Something like:
> > `fp = io.TextIOWrapper(archive.open(filename))`. Then, we can pass this new
> > `fp` object to `self._parse_ini_file`. There are two places where we call
> > `archive.open` so you'll need to repeat this in both places.
> >
> 
> I have made the suggested changes in mozversion.py but I cannot find the
> occurrence of archive.open , could you please help me?
> I tried ```grep -roh archive.open . | wc -w``` in the mozilla-unified
> folder, but it did not return any result.

Both occurrences of calls to 'archive.open' are within mozversion.py, you can see them here: https://searchfox.org/mozilla-central/rev/5786b9be9f887ed371c804e786081b476a403104/testing/mozbase/mozversion/mozversion/mozversion.py#66,73

Comment 102

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I'm going to take this as I have a patch ready.

Comment 103

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Bug 1471920 - Fix Python 3 support in mozversion; r?ahal

Comment 104

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Here's a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c63f182bba793c406b20c4a72f2bd59ac96b7c46

Comment 105

[Dave Hunt [:davehunt] [he/him] ⌚BST]

There are two remaining issues I see:

The first one is that the mozversion tests depend on a mozbuild fixture for obtaining a real binary path. At this time mozbuild doesn't support Python 3, and when I looked into what changes would be needed I stopped when I hit a mozprocess issue as that's being taken care of in bug 1428713. For now test_real_binary will be skipped in Python 3, however we can either address this here if bug 1428713 is resolved soon and the remaining mozbuild changes are simple, or we can follow this up in another bug that's blocked by bug 1473499.

The second issue is that mozversion depends on mozlog, and we don't yet have a fully Python 3 compatible release of mozlog. Ideally we'd wait for bug 1471648 to be resolved and update our dependency in mozversion before pushing out a new release.

Let's see if there's any movement on these dependencies before releasing a new version of mozversion.

Comment 106

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Dave Hunt [:davehunt] [he/him] ⌚️UTC from comment #105)

The second issue is that mozversion depends on mozlog, and we don't yet have a fully Python 3 compatible release of mozlog. Ideally we'd wait for bug 1471648 to be resolved and update our dependency in mozversion before pushing out a new release.

mozlog 4.0.0 has been released with support for Python 3.

Setting myself as needinfo to check regarding mozbuild/mozprocess

Comment 107

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6837617a1024
Fix Python 3 support in mozversion; r=ahal

Comment 108

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6837617a1024

Comment 109

[Dave Hunt [:davehunt] [he/him] ⌚BST]

mozversion 2.1.0 has been released to PyPI: https://pypi.org/project/mozversion/2.1.0/