Last Comment Bug 1189858 - [mozprofile] investigate why mozprofile requires manifestparser, and try to remove that
: [mozprofile] investigate why mozprofile requires manifestparser, and try to r...
Status: RESOLVED FIXED
: leave-open
Product: Testing
Classification: Components
Component: Mozbase (show other bugs)
: Trunk
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: Julien Pagès (:parkouss)
:
:
Mentors:
Depends on:
Blocks: 1190334 1190287
  Show dependency treegraph
 
Reported: 2015-07-31 11:13 PDT by Julien Pagès (:parkouss)
Modified: 2015-08-06 12:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
affected


Attachments
1189858.patch (3.78 KB, patch)
2015-08-03 01:23 PDT, Julien Pagès (:parkouss)
ahalberstadt: review+
Details | Diff | Splinter Review
1189858_fix.patch (1.06 KB, patch)
2015-08-05 04:30 PDT, Julien Pagès (:parkouss)
ahalberstadt: review+
Details | Diff | Splinter Review

Description User image Julien Pagès (:parkouss) 2015-07-31 11:13:11 PDT
mozprofile requires manifestparser, and that dependency doesn't make any sense intuitively.

I think we should have a look and try to fix this.
Comment 1 User image Julien Pagès (:parkouss) 2015-08-03 01:23:24 PDT
Created attachment 8642281 [details] [diff] [review]
1189858.patch

The manifestparser dependency of mozprofile is made optionnal with this patch.
It is still possible to install it using the extra pip/setuptools syntax:

> pip install mozprofile[manifest]

Note that the following:

> pip install mozprofile

won't install nor requires manifestparser now.

I forgot to say this in the first comment, but it will be good for outside tools like mozregression to not require this as it is useless for them.

Note that I updated the mozprofile version in the same patch, so I could use that version from mozregression.

I don't think this would break internal tests, as everything in mozbase is zipped and accessible I think. Anyway, we'll see that on try.
Comment 2 User image Andrew Halberstadt [:ahal] 2015-08-04 13:55:21 PDT
Comment on attachment 8642281 [details] [diff] [review]
1189858.patch

Review of attachment 8642281 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! Lgtm.

::: testing/mozbase/mozprofile/setup.py
@@ +30,5 @@
>        license='MPL 2.0',
>        packages=['mozprofile'],
>        include_package_data=True,
>        zip_safe=False,
> +      install_requires=['mozfile >= 1.0', 'mozlog >= 3.0'],

Please keep the deps variable, it's easier to find at the top of the file and by now everyone is trained to look for it there :).
Comment 3 User image Julien Pagès (:parkouss) 2015-08-04 14:21:52 PDT
Thanks Andrew! Fixed, and pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7abe1e52326
Comment 5 User image Julien Pagès (:parkouss) 2015-08-05 00:08:02 PDT
Leave-open so I can release that to pypi once landed in m-c.
Comment 6 User image Carsten Book [:Tomcat] 2015-08-05 04:15:57 PDT
https://hg.mozilla.org/mozilla-central/rev/a21947fb660f
Comment 7 User image Julien Pagès (:parkouss) 2015-08-05 04:30:31 PDT
Created attachment 8643654 [details] [diff] [review]
1189858_fix.patch

Oh, I tested before doing the release - and I made a typo in the last patch...

Sorry for that - this new patch fixes it, I tested it locally with success (specifying or not with pip the extra dependency).
Comment 9 User image Ryan VanderMeulen [:RyanVM] 2015-08-06 12:33:33 PDT
https://hg.mozilla.org/mozilla-central/rev/d00d3d7ffd7d
Comment 10 User image Julien Pagès (:parkouss) 2015-08-06 12:46:08 PDT
Uploaded on pypi:

Submitting dist/mozprofile-0.26.tar.gz to https://pypi.python.org/pypi
Server response (200): OK

Note You need to log in before you can comment on or make changes to this bug.