Bug 1352650 - rubygem-sprockets: Build failure because of a lack of rubygem-therubyracer
Summary: rubygem-sprockets: Build failure because of a lack of rubygem-therubyracer
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: rubygem-sprockets
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jun Aruga
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-07-04 14:42 UTC by Jun Aruga
Modified: 2016-07-08 10:13 UTC (History)
2 users (show)

Fixed In Version: rubygem-sprockets-3.6.3-1.fc25
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-07-08 10:13:48 UTC
Type: Bug


Attachments (Terms of Use)
Fix a JavaScript runtime issue (2.48 KB, patch)
2016-07-07 10:53 UTC, Jun Aruga
no flags Details | Diff

Description Jun Aruga 2016-07-04 14:42:37 UTC
Description of problem:

When I tried to do mock build, that was failed.
Because depended rubygem-therubyracer was retired.
We can fix this issue to use nodejs instead of the rubygem-therubyracer.

Version-Release number of selected component (if applicable):
3.2.0

How reproducible:

Steps to Reproduce:
1. mock -r fedora-rawhide-x86_64 rubygem-sprockets-3.2.0-3.fc25.src.rpm

Actual results:
It is failed with "No package found for: rubygem(therubyracer)"

Expected results:
It is succeed.


Additional info:
None

Comment 1 Jun Aruga 2016-07-04 14:43:32 UTC
I am going to fix it.

Comment 2 Vít Ondruch 2016-07-04 14:44:10 UTC
(In reply to Jun Aruga from comment #0)
> When I tried to do mock build, that was failed.
> Because depended rubygem-therubyracer was retired.
> We can fix this issue to use nodejs instead of the rubygem-therubyracer.

Yes, sure ... that was always the intention.

Comment 3 Jun Aruga 2016-07-04 17:31:18 UTC
Hi, Vit

I have a question to fix this issue.

I found the rubygem-sprockets depends on rubygem-closure-compiler actually.
And it is commented out, because rubygem-closure-compiler had not been created 
yet at that time. but Now it has created.


But I got one problem to use it.
http://pkgs.fedoraproject.org/cgit/rpms/rubygem-sprockets.git/tree/rubygem-sprockets.spec
L68-72


So, I thought I wanted to use it now.
I got an error to build it with rubygem-closure-compiler.
Because the Autoload::Closure::COMPILER_VERSION is commented out in rubygem-closure-compiler. But sprockets is using the variable.

https://github.com/rails/sprockets/blob/v3.6.3/lib/sprockets/closure_compressor.rb#L39

So, which way do you like to fix this?
I have 3 ideas to fix this.

1. Modify lib/sprockets/closure_compressor.rb in rubygem-sprockets.spec to replace COMPILER_VERSION comment out logic, for example replace to COMPILER_VERSION = ''
2. Modify rubygem-closure-compiler.spec to add replacement logic to replace the logic in lib/sprockets/closure_compressor.rb .
3. Keep comment out of rubygem-sprockets.spec L68-72 logic. Do not load rubygem-closure-compiler.

Comment 4 Jun Aruga 2016-07-04 17:34:31 UTC
> Because the Autoload::Closure::COMPILER_VERSION is commented out in rubygem-closure-compiler.

http://pkgs.fedoraproject.org/cgit/rpms/rubygem-closure-compiler.git/tree/rubygem-closure-compiler.spec L51

Comment 5 Vít Ondruch 2016-07-07 08:24:23 UTC
If I understand it right, then I'd opt for the 3rd option, i.e. keep the closure-compiler tests commented out ATM. If you could open new ticket with subject like "Enable closure-compiler test suite" for sprockets to address this later, that would be nice.

Comment 6 Jun Aruga 2016-07-07 08:48:41 UTC
(In reply to Vít Ondruch from comment #5)
> If I understand it right, then I'd opt for the 3rd option, i.e. keep the
> closure-compiler tests commented out ATM. If you could open new ticket with
> subject like "Enable closure-compiler test suite" for sprockets to address
> this later, that would be nice.

Yes, I am going to proceed on 3rd option.
I have created new ticket for this issue.
https://bugzilla.redhat.com/show_bug.cgi?id=1353473

Comment 7 Jun Aruga 2016-07-07 10:53:34 UTC
Created attachment 1177249 [details]
Fix a JavaScript runtime issue

Hi,

I fixed a JavaScript runtime issue. (rhbz#1352650)
I updated to Sprockets 3.6.3.

Also
- Dependency for rawhide, f24: ok.
- Install: ok
- Rpmlint: ok
- License check: ok

I am going to patch this to rawhide and f24.
Could you check my patch?
Thanks.

Comment 8 Jun Aruga 2016-07-07 11:05:15 UTC
Sorry I made a mistake. f24 is no problem.
I am going to patch this to only rawhide.

(In reply to Jun Aruga from comment #7)
> Created attachment 1177249 [details]
> Fix a JavaScript runtime issue
> 
> Hi,
> 
> I fixed a JavaScript runtime issue. (rhbz#1352650)
> I updated to Sprockets 3.6.3.
> 
> Also
> - Dependency for rawhide, f24: ok.
> - Install: ok
> - Rpmlint: ok
> - License check: ok
> 
> I am going to patch this to rawhide and f24.
> Could you check my patch?
> Thanks.

Comment 9 Vít Ondruch 2016-07-07 11:22:22 UTC
(In reply to Jun Aruga from comment #7)
> Could you check my patch?

Looks quite OK to me, but I would use probably:

BuildRequires: /usr/bin/node

i.e. I would depend on the executable instead of the nodejs package name, because the executable is what the ExecJS requires [1] at the end.





[1] https://github.com/rails/execjs/blob/master/lib/execjs/runtimes.rb#L23

Comment 10 Jun Aruga 2016-07-07 12:44:33 UTC
Hi, Vit
Thanks for the review.

Okay, I will use your way, and push it to master branch.
And thank for your adding ACL commits to me in PkgDB. I forgot to apply it.


(In reply to Vít Ondruch from comment #9)
> (In reply to Jun Aruga from comment #7)
> > Could you check my patch?
> 
> Looks quite OK to me, but I would use probably:
> 
> BuildRequires: /usr/bin/node
> 
> i.e. I would depend on the executable instead of the nodejs package name,
> because the executable is what the ExecJS requires [1] at the end.
> 
> 
> 
> 
> 
> [1] https://github.com/rails/execjs/blob/master/lib/execjs/runtimes.rb#L23

Comment 11 Jun Aruga 2016-07-07 12:59:00 UTC
I am going to use 

BuildRequires: %{_bindir}/node

instead of

> BuildRequires: /usr/bin/node


It looks better.


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