Bug 1357592 - rubygem-execjs: FTBFS
Summary: rubygem-execjs: FTBFS
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: rubygem-execjs
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-18 15:23 UTC by Jun Aruga
Modified: 2016-07-25 14:03 UTC (History)
1 user (show)

Fixed In Version: rubygem-execjs-2.7.0-1.fc25
Clone Of:
Environment:
Last Closed: 2016-07-25 14:03:48 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Fix FTBFS. Update to 2.7.0. (2.14 KB, patch)
2016-07-19 12:10 UTC, Jun Aruga
no flags Details | Diff
Fix FTBFS. Update to 2.7.0. (24.02 KB, patch)
2016-07-22 11:19 UTC, Jun Aruga
no flags Details | Diff
Fix FTBFS. Update to 2.7.0. (24.03 KB, patch)
2016-07-22 15:24 UTC, Jun Aruga
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1114345 0 unspecified CLOSED rubygem-execjs-2.7.0 is available 2021-02-22 00:41:40 UTC

Internal Links: 1114345

Description Jun Aruga 2016-07-18 15:23:34 UTC
Description of problem:
FTBFS(Fails to build from source) because of retired rubygem-therubyracer.

Version-Release number of selected component (if applicable):
rubygem-execjs-2.2.0-3.fc25

How reproducible:


Steps to Reproduce:
1. $ mock -r fedora-rawhide-x86_64 rubygem-execjs-2.2.0-3.fc25.src.rpm

Actual results:
The test is failed with the message:
No matching package to install: 'rubygem(therubyracer)'
Not all dependencies satisfied
Error: Some packages could not be found.

Expected results:
The test is succeeded.

Comment 1 Jun Aruga 2016-07-19 12:10:31 UTC
Created attachment 1181612 [details]
Fix FTBFS. Update to 2.7.0.

Hi,
I fixed FTBFS, and updated to 2.7.0 (latest version).
Could you check my patch for that?


# Highlight

May I ask you a question?

execjs/support/json2.js
looks bundled.

Do we need this comment in the RPM spec file?
https://fedoraproject.org/wiki/Packaging:Guidelines
Provides: bundled(<libname>) = <version>


The dependency for the other packages is no problem.

Comment 2 Vít Ondruch 2016-07-19 12:57:11 UTC
(In reply to Jun Aruga from comment #1)
> execjs/support/json2.js
> looks bundled.
> 
> Do we need this comment in the RPM spec file?
> https://fedoraproject.org/wiki/Packaging:Guidelines
> Provides: bundled(<libname>) = <version>

Hmm, this would be one option (historically, there were some exceptions for bundling of JS).

However, looking at the history of the file [1], it seems that the best option could be to drop the file entirely, e.g. something like revert of commit [2]. We don't need to support WSH, since there is no way how to get this part of the code working on Fedora anyway. What do you think?



[1] https://github.com/rails/execjs/commits/master/lib/execjs/support/json2.js
[2] https://github.com/rails/execjs/commit/8b22842cbd3e2cf46d413aa7d352869b19303d31

Comment 3 Vít Ondruch 2016-07-19 13:01:01 UTC
Also, there is js-json package in fedora, which appears to ship json2, so in theory, this could be unbundled, but there are some upstream modifications to this file :/

Comment 4 Jun Aruga 2016-07-19 16:35:13 UTC
(In reply to Vít Ondruch from comment #2)
> (In reply to Jun Aruga from comment #1)
> > execjs/support/json2.js
> > looks bundled.
> > 
> > Do we need this comment in the RPM spec file?
> > https://fedoraproject.org/wiki/Packaging:Guidelines
> > Provides: bundled(<libname>) = <version>
> 
> Hmm, this would be one option (historically, there were some exceptions for
> bundling of JS).
> 
> However, looking at the history of the file [1], it seems that the best
> option could be to drop the file entirely, e.g. something like revert of
> commit [2]. We don't need to support WSH, since there is no way how to get
> this part of the code working on Fedora anyway. What do you think?
> 
> 
> 
> [1]
> https://github.com/rails/execjs/commits/master/lib/execjs/support/json2.js
> [2]
> https://github.com/rails/execjs/commit/
> 8b22842cbd3e2cf46d413aa7d352869b19303d31

Hi,
In this case, I like to drop the json2.js file too.
the json2.js is called from only JScript logic.
And JScript is not available on Fedora right now.

The logic is when the listed binary commands exists, JScript is available.
But actually not in Fedora.
https://github.com/rails/execjs/blob/master/lib/execjs/runtimes.rb#L43

So, I tried below way in the build section. and passed the test.
+rm -f lib/execjs/support/json2.js
+sed -i '/files/ s|"lib/execjs/support/json2.js", ||' %{gem_name}.gemspec

Then
Update the rubygem-execjs.spec
License: MIT and Public Domain
to 
License: MIT


About the upstream,
below json2.js latest version [1] looks fixed the execjs original modification.
So, I am going to suggest the execjs upstream to use [1], not bundle it in the execjs, directly.

[1] https://github.com/douglascrockford/JSON-js/blob/master/json2.js
[2] https://github.com/rails/execjs/commit/f47c02c8536ce7cf0850b6a511fcecad6539eeaf

Is it ok for this way?

Comment 5 Vít Ondruch 2016-07-20 13:11:40 UTC
(In reply to Jun Aruga from comment #4)
> (In reply to Vít Ondruch from comment #2)
> > (In reply to Jun Aruga from comment #1)
> > > execjs/support/json2.js
> > > looks bundled.
> > > 
> > > Do we need this comment in the RPM spec file?
> > > https://fedoraproject.org/wiki/Packaging:Guidelines
> > > Provides: bundled(<libname>) = <version>
> > 
> > Hmm, this would be one option (historically, there were some exceptions for
> > bundling of JS).
> > 
> > However, looking at the history of the file [1], it seems that the best
> > option could be to drop the file entirely, e.g. something like revert of
> > commit [2]. We don't need to support WSH, since there is no way how to get
> > this part of the code working on Fedora anyway. What do you think?
> > 
> > 
> > 
> > [1]
> > https://github.com/rails/execjs/commits/master/lib/execjs/support/json2.js
> > [2]
> > https://github.com/rails/execjs/commit/
> > 8b22842cbd3e2cf46d413aa7d352869b19303d31
> 
> Hi,
> In this case, I like to drop the json2.js file too.
> the json2.js is called from only JScript logic.
> And JScript is not available on Fedora right now.
> 
> The logic is when the listed binary commands exists, JScript is available.
> But actually not in Fedora.
> https://github.com/rails/execjs/blob/master/lib/execjs/runtimes.rb#L43
> 
> So, I tried below way in the build section. and passed the test.
> +rm -f lib/execjs/support/json2.js

The '-f' is not a good approach, since you probably want to fail early when the file is missing.

> +sed -i '/files/ s|"lib/execjs/support/json2.js", ||' %{gem_name}.gemspec

Anyway, yes, this is possible solution. But have you tried to revert the commit I referenced above? (actually you would need to revert also [1]). I slightly prefer the revert, since if would completely remove the notion of JScript, which looks to be cleaner approach ...

> Then
> Update the rubygem-execjs.spec
> License: MIT and Public Domain
> to 
> License: MIT

Yes ...


> About the upstream,
> below json2.js latest version [1] looks fixed the execjs original
> modification.

Do I understand correctly that you are saying that the upstream version includes the changes made by execjs guys? Because I disagree with that. The upstream version does not contains the modifications done by commit [1] IMO.

> So, I am going to suggest the execjs upstream to use [1], not bundle it in
> the execjs, directly.

I am sorry, but I can't see any way, how this would be achievable in upstream. We can do that for Fedora and use the js-json package to unbundle the json2.js, but upstream has not the possibility. Or am I missing something?


[1] https://github.com/rails/execjs/commit/f47c02c8536ce7cf0850b6a511fcecad6539eeaf#diff-7fcbc22d100e54a44ab876955494969a

Comment 6 Jun Aruga 2016-07-21 18:56:27 UTC
My next action for json2.js depends on the result of below pull-request to the upstream. I am going to wait until Sunday (July 24th).

https://github.com/rails/execjs/pull/49


As a result of my pull-request, they may delete their own modification for the json2.js from the repository.
They may update bundled json2.js to latest version without their own modification.
They may delete the JScript logic.

If they do nothing until Sunday, I will delete all the JScript logic in the execjs with a patch using your way "revert to commit".

Comment 7 Jun Aruga 2016-07-22 11:19:09 UTC
Created attachment 1182821 [details]
Fix FTBFS. Update to 2.7.0.

Hi,

(In reply to Jun Aruga from comment #6)
> If they do nothing until Sunday, I will delete all the JScript logic in the
> execjs with a patch using your way "revert to commit".

I created new patch to fix this issue using "revert commit", and delete all the JScript logic.
I did not wait the upstream's action this time.

Could you check my patch again?

Thanks.

Comment 8 Jun Aruga 2016-07-22 15:24:57 UTC
Created attachment 1182869 [details]
Fix FTBFS. Update to 2.7.0.

Sorry I uploaded the patch file again, because of below additional modification.

-# Public Domain: %%{gem_libdir}/execjs/support/json2.js
-License: MIT and Public Domain
+License: MIT

Comment 9 Vít Ondruch 2016-07-25 11:08:40 UTC
The patch looks good. The only nit is that I would not reference your personal repository:

https://github.com/junaruga/execjs/commit/d4fd415

One day you will do cleanup or remove the repository for whatever reason and the link will become invalid. The description how to prepare such patch would be enough in this case.

Comment 10 Jun Aruga 2016-07-25 14:03:31 UTC
> The patch looks good. The only nit is that I would not reference your personal repository:
> 
> https://github.com/junaruga/execjs/commit/d4fd415

Okay, I removed the line from the RPM spec file, and pushed to RAWHIDE.


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