Closed Bug 1189858 Opened 6 years ago Closed 6 years ago

[mozprofile] investigate why mozprofile requires manifestparser, and try to remove that

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(firefox42 affected)

RESOLVED FIXED
Tracking Status
firefox42 --- affected

People

(Reporter: parkouss, Assigned: parkouss)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

mozprofile requires manifestparser, and that dependency doesn't make any sense intuitively.

I think we should have a look and try to fix this.
Attached patch 1189858.patchSplinter Review
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.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8642281 - Flags: review?(ahalberstadt)
Blocks: 1190287
Blocks: 1190334
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 :).
Attachment #8642281 - Flags: review?(ahalberstadt) → review+
Leave-open so I can release that to pypi once landed in m-c.
Keywords: leave-open
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).
Attachment #8643654 - Flags: review?(ahalberstadt)
Attachment #8643654 - Flags: review?(ahalberstadt) → review+
Uploaded on pypi:

Submitting dist/mozprofile-0.26.tar.gz to https://pypi.python.org/pypi
Server response (200): OK
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.