Bug 1883732

Note that te test suite is troublesome at this moment. I didn't manage to make the tests work, due to paths (assets not found, or even SassC). And CSS 'compressor' doesn't seem to work. I will try to resolve those issues upstream.

I'm taking this for a review.

* Weird RexExp
  - The 'g' in the following RegExp is probably not needed, since you are matching single line anyway:

~~~
sed -i -e '/require .pry./ s/^/#/g' test/test_helper.rb
~~~

* Test suite
  - It seems you got completely drown. If you started with the rubygem-sass-rails, it would be easier. This is where I got:

~~~
... snip ...

BuildRequires: rubygem(bundler)
BuildRequires: rubygem(mocha)
BuildRequires: rubygem(railties)
BuildRequires: rubygem(sassc)
BuildRequires: rubygem(sprockets-rails)
BuildRequires: rubygem(tilt)

... snip ...

%check
pushd .%{gem_instdir}
# Copy in .gemspec and use the sass-rails sources
cp %{buildroot}%{gem_spec} sassc-rails.gemspec

# Avoid unnecessary dependency
sed -i -e '/require .pry./ s/^/#/g' test/test_helper.rb
sed -i -e '/dependency.*pry./ s/^/#/' sassc-rails.gemspec

sed -i -e '/dependency.*rake./ s/^/#/' sassc-rails.gemspec

ruby -e 'Dir.glob "./test/**/*.rb", &method(:require)'
popd

... snip ...
~~~

  - BTW some of the failures could be, because you have required `rubygem(sass)` instead of `rubygem(sassc)` (note the 'c' difference).

* Licensing
  - Please check the license of test/dummy/app/assets/stylesheets/erb_render_with_context.css.erb. It seems
    the package should include the `OFL` license. But probably better to check with upstream.

Ping?

Hello,

sorry for the delay. Unfortunately, I forgot to continue to work on this, after getting initially stuck with the test suite. Now I was able to debug it, and all the tests pass.
I've a also fixed the license for the font (interesting is that licensecheck didn't find it). 

Resulting in changes on top of original spec: https://git.io/JTiyQ

Up-to-date Copr build (also has spec file): https://copr.fedorainfracloud.org/coprs/build/1722749

I've also run some automated checks:
  - Tests: ok
  - Syntax check: ok
  - Dependent packages: ok
  - Smoke test: ok
  - rpmlint: ok (false positives)
_ _ _ _

Test log: cpr/rubygem-sassc-rails_test.log
gem2rpm diff: cpr/rubygem-sassc-rails_gem2rpm.diff

_ _ _ _

Additionaly:

> * Weird RexExp
>   - The 'g' in the following RegExp is probably not needed, since you are matching single line anyway:

Doesn't make much difference then? I'm used to write regexes like that... (removed)

> * Test suite
>   - It seems you got completely drown. If you started with the rubygem-sass-rails, it would be easier. This is where I got:

Yes, I was in a hurry to have it built :) ... (weird that I forgot afterwards).

Ping?

Sorry, this is still on my TODO list

Just to make it clear, I am reviewing this .spec file and SRPM:

https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01722749-rubygem-sassc-rails/rubygem-sassc-rails.spec
https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01722749-rubygem-sassc-rails/rubygem-sassc-rails-2.1.2-1.3.fc34.src.rpm

* Requiring sass-rails
  - I wonder is the `BuildRequires: rubygem(sass-rails)` really needed? If the answer is yes, then you should do something about this:

~~~
 Problem: conflicting requests
  - nothing provides (rubygem(railties) >= 4.0.0 with rubygem(railties) < 6) needed by rubygem-sass-rails-5.0.7-7.fc33.noarch
(try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages)
~~~

    Obviously, we still have rubygem-sass-rails, which was not build for Rails 6+. So you need bootstrap or rethink the BRs.

(In reply to Vít Ondruch from comment #9)
> Just to make it clear, I am reviewing this .spec file and SRPM:

Yes, that's the .spec file I was pointing to, for review.


>     Obviously, we still have rubygem-sass-rails, which was not build for
> Rails 6+. So you need bootstrap or rethink the BRs.


I've built rubygem-sass-rails in the side-tag f34-build-side-32997:

  https://koji.fedoraproject.org/koji/taskinfo?taskID=56151529

Yes, you're right, It's needed:

```nothing provides (rubygem(sassc-rails) >= 2.1 with rubygem(sassc-rails) < 3 with rubygem(sassc-rails) >= 2.1.1) needed by rubygem-sass-rails-6.0.0-1.fc34.noarch```

I've added appropriate bcond_with macros. I opted to add them for this package, as only build phase is affected.

Change:  https://git.io/JkPvI
Spec:    https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01785216-rubygem-sassc-rails/rubygem-sassc-rails.spec
SRPM:    https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01785216-rubygem-sassc-rails/rubygem-sassc-rails-2.1.2-1.4.fc34~bootstrap.src.rpm

Koji build (in side-tag): https://koji.fedoraproject.org/koji/taskinfo?taskID=56152403
COPR build:               https://copr.fedorainfracloud.org/coprs/build/1785216

Note: For real build I'll reset the release to `1`, off course.

* Hidden files:
  - There is a hidden file reported:

~~~
rubygem-sassc-rails-doc.noarch: W: hidden-file-or-dir /usr/share/gems/gems/sassc-rails-2.1.2/test/dummy/app/assets/images/.keep
~~~

  - Given its nature, this is probably not a big deal.

> Note: For real build I'll reset the release to `1`, off course. 

Just FTR, rpmlint complains about "incoherent-version-in-changelog 2.1.2-1.3 ['2.1.2-1.4.fc34', '2.1.2-1.4']" but I guess you are going to fix this.

Otherwise, LGTM => I APPROVE this package.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rubygem-sassc-rails

Thank you for the review!