Bug 1357879
| Summary: | rubygem-uglifier: FTBFS | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jun Aruga <jaruga> | ||||||||
| Component: | rubygem-uglifier | Assignee: | Jun Aruga <jaruga> | ||||||||
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
| Severity: | unspecified | Docs Contact: | |||||||||
| Priority: | unspecified | ||||||||||
| Version: | 25 | CC: | strzibny, vondruch | ||||||||
| Target Milestone: | --- | ||||||||||
| Target Release: | --- | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Whiteboard: | |||||||||||
| Fixed In Version: | rubygem-uglifier-3.0.0-1.fc25, rubygem-uglifier-3.0.0-1.fc26 | Doc Type: | If docs needed, set a value | ||||||||
| Doc Text: | Story Points: | --- | |||||||||
| Clone Of: | Environment: | ||||||||||
| Last Closed: | 2016-08-01 10:06:14 UTC | Type: | Bug | ||||||||
| Regression: | --- | Mount Type: | --- | ||||||||
| Documentation: | --- | CRM: | |||||||||
| Verified Versions: | Category: | --- | |||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||
| Embargoed: | |||||||||||
| Bug Depends On: | 1361179 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Jun Aruga
2016-07-19 12:58:44 UTC
I will fix it. Created attachment 1182133 [details]
Fix FTBFS, update to 3.0.0
Hi,
I fixed the FTBFS. and updated to 3.0.0.
# Highlight
- The dependency is no problem.
- This time, when I updated from v2.4.0 to v3.0.0.
I removed json dependency from the RPM spec file.
because then below json dependency is removed from the gem spec file.
```
spec.add_runtime_dependency "json", ">= 1.8.0"
```
- I did not touch bundled Javascript files this time.
Could you check my patch?
Thanks.
Created attachment 1182138 [details]
Fix FTBFS, update to 3.0.0
Sorry, I will upload the patch file again.
(In reply to Jun Aruga from comment #2) > - This time, when I updated from v2.4.0 to v3.0.0. > I removed json dependency from the RPM spec file. > because then below json dependency is removed from the gem spec file. You should also remove the "Requires: rubygem(execjs)", since this should be autogenerated (and I think RPM or rpmlint compalins about duplicated Requires). > - I did not touch bundled Javascript files this time. Would you mind to explore, if there is a chance to undbundle the bundled JS in favor of uglify-js package? (In reply to Vít Ondruch from comment #4) > (In reply to Jun Aruga from comment #2) > > - This time, when I updated from v2.4.0 to v3.0.0. > > I removed json dependency from the RPM spec file. > > because then below json dependency is removed from the gem spec file. > > You should also remove the "Requires: rubygem(execjs)", since this should be > autogenerated (and I think RPM or rpmlint compalins about duplicated > Requires). If you want to remove the line "Requires: rubygem(execjs)", I want to remove not only "Requires: rubygem(execjs)" but also below 3 lines. Requires: ruby(release) Requires: ruby(rubygems)- Requires: rubygem(execjs) >= 0.3.0 The 3 lines are not needed when comparing the result of gem2rpm for the uglifier-3.0.0.gem. Though my rpmlint did not complain about the duplicated Requires, I will take cafe of it. $ rpmlint --version rpmlint version 1.8 Copyright (C) 1999-2007 Frederic Lepied, Mandriva $ rpmlint -i ./*.spec /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm rubygem-uglifier.noarch: W: spelling-error %description -l en_US minifies -> miniseries The value of this tag appears to be misspelled. Please double-check. rubygem-uglifier.noarch: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. rubygem-uglifier.src: W: spelling-error %description -l en_US minifies -> miniseries The value of this tag appears to be misspelled. Please double-check. 3 packages and 1 specfiles checked; 0 errors, 3 warnings. > > - I did not touch bundled Javascript files this time. > > Would you mind to explore, if there is a chance to undbundle the bundled JS > in favor of uglify-js package? Okay I will explore to unbundle it. > Okay I will explore to unbundle it. It is difficult for us to unbundle uglity-js right now. Because The rubygem-uglifier one of the file lib/uglify.js is generated from source-map and uglify-js. We have nodejs-source-map package for the source-map and the uglify-js package. However we do not have nodejs-source-map's dist file (dist/source-map.js) in the nodejs-source-map package https://github.com/lautis/uglifier/blob/v3.0.0/Rakefile#L22 So, I will ask the owner of nodejs-source-map to put and include dist/source-map.js in the package. This bug appears to have been reported against 'rawhide' during the Fedora 25 development cycle. Changing version to '25'. Created attachment 1184661 [details] Fix FTBFS, update to 3.0.0 Hi, I created again to unbundle uglify-js in the package, and use system uglify-js. However this package needs uglify-js pacakge version 2.7.0. - Fix for FTBFS. (rhbz#1357879) - Update to uglifier 3.0.0 - Use system uglify-js instead of bundled one. I used BuildRequires: uglify-js >= 2.7.0 instead of BuildRequires: %{_bindir}/uglifyjs Because I wanted to specify the version of it. The SRPM is here. https://copr.fedorainfracloud.org/coprs/vondruch/ror5/package/rubygem-uglifier/ Could you check the patch again? Thanks. Sorry below 2 lines in the spec file, is not needed. I will remove these 2 lines after your review.
> BuildRequires: nodejs-packaging
> BuildRequires: npm(source-map)
Several notes: * First of all, minor nit. If you are linking the GH PR above the Patch line, please avoid the ".patch" suffix. That way, one can easily get the whole picture. * You don't have to be that wordy in comments above license. I'd go with: "# lib/source-map.js - BSD" and that is it. The rest of the comments should go to "Provides: bundled(nodejs-source-map)", which you are missing. * You are removing the es5.js and split.js, but I don't think the explanation is sufficient. I would suggest something like: "Unbundling es5.js and split.js files, since they are needed only by JScript engine, which is not supported on Fedora". Does it explain what you are trying to achieve? *** Unbundling Unfortunately, this: ``` # Use sysmtem uglify.js uglifyjs --self --comments /Copyright/ > lib/uglify.js ``` is not what I consider unbundling, since: 1) There is no runtime dependency on js-uglify (uglify-js). 2) Update of uglify-js does not result in update of the lib/uglify.js Thank you for your review. > Several notes: Okay, I will update the spec file for that. > *** Unbundling uglify-js.js was bundled on the uglify-js package at past time. But now not. https://bugzilla.redhat.com/show_bug.cgi?id=894725 So, I am asking the upstream to include it again. https://github.com/mishoo/UglifyJS2/issues/1239 I will also update below page too for "Provides: bundled(nodejs-source-map)". https://fedoraproject.org/wiki/Bundled_Libraries_Virtual_Provides I pushed to master(rawhide). Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=15048704 But I could not push to f25 branch because uglify-js is still not updated to 2.7.0. So, I asm asking to the upsatream to update to 2.7.0 on f25 branch too. https://bugzilla.redhat.com/show_bug.cgi?id=1361179 After that, I will push it to f25 branch too. Finally I pushed it to f25 branch too. |