One note: you want to depend on epiphany-runtime >= 3.20 rather than epiphany >= 3.20. That allows users who prefer other browsers to uninstall the Epiphany desktop file and search provider, while leaving the binary around to handle web apps. This way it seems to the user as if Epiphany is not installed.

We need to wait for upstream to include fix for https://bugzilla.gnome.org/show_bug.cgi?id=767101 before getting this to Fedora.

https://bugzilla.gnome.org/show_bug.cgi?id=767101 is fixed. Is an update needed here?

I had lot of things to do somewhere else recently.

I'll check status of this during this weekend.

Updated SPEC: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-26-x86_64/00578114-fedora-developer-portal/fedora-developer-portal.spec

Updated SRPM: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-26-x86_64/00578114-fedora-developer-portal/fedora-developer-portal-0.9.3-0.1.git167ae09.fc26.src.rpm


I've refactored content fetching so it now no longer depends on "3rd party" (mine) repo with generated static site but actually uses data from Source0 archive and runs jekyll/wget to compile static site during packaging to rpm. 

This is possible since jekyll is packaged in Fedora but two gems are not:
*jekyll-lunr-js-search
*jekyll-sitemap

I'll look into this during July and create additional Review Requests for these gems.

You can build this srpm with rpmbuild but not in mock (gem fetching is failing there because of missing internet access.)

If you are still looking for a sponsor, I can do it — let me know if you are interested. It would be great if you could do (informal) reviews of two or three other packages from http://fedoraproject.org/PackageReviewStatus/NEW.html.

Please submit the additional reviews and add them as "Depends on" here.

Some notes on this package:
- %description is supposed to say what the package does. It currently reads like an advertisement ;) Please include some down-to-earth description.

> kill -9 `ps aux | grep jekyll | awk '{print $2}'` || :

Yikes. You're assuming that there's just one jekyll running on the machine under this user. That's super brittle. There also shouldn't be any need to use -9.

Maybe something like this instead:

jekyll serve &
...
kill $!

The desktop file should be checked with desktop-file-validate or desktop-file-install, see https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> If you are still looking for a sponsor, I can do it — let me know if you are
> interested.
Sure, thanks!

(In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> It would be great if you could do (informal) reviews of two or three other 
> packages from http://fedoraproject.org/PackageReviewStatus/NEW.html.
Ok, I can try it, should I just add comments with review and nothing else?

I should have additional review requests ready by tomorrow if there are not many issues (I am not very experienced packager after all... :) ).

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> jekyll serve &
> ...
> kill $!

This didn't work (kill tried to kill completely different pid, not sure why though). But this is working just fine:
jekyll serve --detach
pkill -f jekyll

Is this solution better? I'll upload updated SPEC later today.

(In reply to František Zatloukal from comment #8)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> > If you are still looking for a sponsor, I can do it — let me know if you are
> > interested.
> Sure, thanks!
> 
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> > It would be great if you could do (informal) reviews of two or three other 
> > packages from http://fedoraproject.org/PackageReviewStatus/NEW.html.
> Ok, I can try it, should I just add comments with review and nothing else?
Yes, look at the spec file and try to find issue and stuff that is wrong
according to the guidelines. It's also good to run fedora-review (it's very
strongly encouraged, especially if you're new to reviews), and look at all
the stuff it says. That gives you a very good idea of what to check for.
fedora-review builds the package in mock, and that is useful to verify
that build dependencies are not missing.

> I should have additional review requests ready by tomorrow if there are not
> many issues (I am not very experienced packager after all... :) ).
Great.

> 
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> > jekyll serve &
> > ...
> > kill $!
> 
> This didn't work (kill tried to kill completely different pid, not sure why
> though). But this is working just fine:
> jekyll serve --detach
> pkill -f jekyll
That's not idea, all solutions which grep the global process list are wrong.
Consider the case where you're doing two parallel builds in mock for amd64 and
i386 on the same machine: there are two jykylls running.

But OK, upload the latest spec file you have, and I'll see if I can patch it
to use $!. Otherwise we can ask for help on fedora-devel… I expect people to have
had the same issue, e.g. in tests.

SPEC: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-25-x86_64/00578717-fedora-developer-portal/fedora-developer-portal.spec

I've added .desktop file checking and new description.

I've played with other options to kill jekyll but with no luck.

So, I've looked at packaging jekyll-sitemap with gem2rpm. It looks like there is some automatic requires detection because it's depending on rubygem-jekyll >= 3.3 (newer than packaged in Fedora). I was able to solve dependency chain on my system and it's pretty complicated:

fedora-developer-portal depends on rubygem-jekyll-sitemap
rubygem-jekyll-sitemap depends on rubygem-jekyll >= 3.3
rubygem-jekyll >= 3.3 depends on rubygem-addressable
rubygem-addressable depends on rubygem-public_suffix (built in rawhide only, but installable in f26)

I will create review request for rubygem-addressable soon, rubygem-jekyll bug regarding new version availability and rubygem-public_suffix bug report to package it even for F26 at least.

URL: should give the full URL to the upstream tarball, so it's possible to build the whole file from the spec file, and to verify that the tarball you have matches the upstream tarball.

The build part (everything starting from "jekyll build") should be in the %build section.

The kill part is indeed tough. My idea of putting 'jekyll serve' in the background does not work because it takes a while for jekyll to open the listening port, and we have to wait for that, but there's not synchronization mechanism. So using '--detach' seems like the only option.

I'd do this:
--------------------------------------------------------------------
diff --git fedora-developer-portal.spec fedora-developer-portal.spec
index a711e0b05b..6ea026e24b 100644
--- fedora-developer-portal.spec
+++ fedora-developer-portal.spec
@@ -49,6 +49,8 @@ touch _includes/announcement.html
 gem install jekyll-lunr-js-search
 gem install jekyll-sitemap
 
+%build
+
 # Build the site and start server
 jekyll build
 jekyll serve --detach
@@ -60,9 +62,7 @@ wget --convert-links -r http://127.0.0.1:4000/ || :
 mv 127.0.0.1\:4000/ fedora-developer-portal-content-%{shortcommit}
 
 # Kill server in the background
-kill -9 `ps aux | grep jekyll | awk '{print $2}'` || :
-
-%build
+pgrep -f 'jekyll serve --detach' | xargs kill
 
 %install
 mkdir -p %buildroot%{_bindir}
---------------------------------------------------------------------

OK, let's concentrate on the deps. This package looks good. I'll add fedora-review+ when the deps are ready.

I've updated SPEC with your changes: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-26-x86_64/00581156-fedora-developer-portal/fedora-developer-portal.spec


Regarding the URL - upstream never really did a release, so the only option would be to use master.zip link provided by Github. There is problem that we wouldn't know shortcommit till archive unzip in %prep (and we need it for Release: which is above %prep).

I can try to contact repo maintainer to do a "release for us".

> so the only option would be to use master.zip link provided by Github.

That's generally OK, except that you link to an archive for a specific commit (master is a moving target, and we need things to be reproducible). This should work:

%global commit b314c8a2ffc643d27da0f6d75981cb784fac8374
%global shortcommit %(c=%{commit}; echo ${c:0:7})
https://github.com/developer-portal/content/archive/%{commit}/fedora-developer-portal-%{shortcommit}.tar.gz

There are guidelines [https://fedoraproject.org/wiki/Packaging:SourceURL#Commit_Revision], but they are slightly out of date, because github now allows nicer syntax. I'll file an FPC bug for that.

Oh, another thing: don't use %define [https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define].

New SPEC: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00583062-fedora-developer-portal/fedora-developer-portal.spec

New SRPM: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00583062-fedora-developer-portal/fedora-developer-portal-0.9.4-0.1.git167ae09.fc27.src.rpm

I've bundled few gems to mitigate rubygem-therubyracer not being built and maintained in latest Fedoras. I think bundling only for build time is OK.

The other option would be to patch upstream jekyll-lunr-js-search to rely for JS on rubygem-execjs instead of rubygem-therubyracer or strip out search completely (which is broken right now anyway in packaged version).

Regarding building the noarch package in Fedora infra - can I rely on x86_64 builder or not? Cause bundled libv8 is architecture-specific (and %ifarch didn't work for me in mock, probably because of noarch package)

(In reply to František Zatloukal from comment #15)
> New SPEC:
> https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-
> portal/fedora-rawhide-x86_64/00583062-fedora-developer-portal/fedora-
> developer-portal.spec
> 
> New SRPM:
> https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-
> portal/fedora-rawhide-x86_64/00583062-fedora-developer-portal/fedora-
> developer-portal-0.9.4-0.1.git167ae09.fc27.src.rpm
> 
> I've bundled few gems to mitigate rubygem-therubyracer not being built and
> maintained in latest Fedoras. I think bundling only for build time is OK.
Sounds reasonable.

> The other option would be to patch upstream jekyll-lunr-js-search to rely
> for JS on rubygem-execjs instead of rubygem-therubyracer or strip out search
> completely (which is broken right now anyway in packaged version).
> 
> Regarding building the noarch package in Fedora infra - can I rely on x86_64
> builder or not? Cause bundled libv8 is architecture-specific (and %ifarch
> didn't work for me in mock, probably because of noarch package)

For a noarch package, you cannot rely on a specific builder architecture,
unless you use ExclusiveArch. See
https://fedoraproject.org/wiki/Packaging:Node.js#ExclusiveArch.
I think those instructions there should work for this case too.

New SPEC: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00583307-fedora-developer-portal/fedora-developer-portal.spec

New SRPM: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00583307-fedora-developer-portal/fedora-developer-portal-0.9.4-0.2.git167ae09.fc27.src.rpm


I've added ExclusiveArch: ix86 x86_64. This can go away when we won't rely on bundling libv8 for building (and package will become noarch).

So, we are now blocked by rubygem-jekyll (and its dependencies - rubygem-liquid, rubygem-addressable) only.

So, I went forward and included unmet dependencies. Builds just fine in F28 mock, I don't intend to push to older Fedora Releases.

SPEC: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00627908-fedora-developer-portal/fedora-developer-portal.spec

SRPM: https://copr-be.cloud.fedoraproject.org/results/frantisekz/fedora-developer-portal/fedora-rawhide-x86_64/00627908-fedora-developer-portal/fedora-developer-portal-0.9.4-0.3.git167ae09.fc28.src.rpm

I can keep an eye on Bundled Build Deps and remove them as they are met in Fedora repos. Is that okay?

It seems you actually need the dependency on jekyll, build fails otherwise.

warning: Macro expanded in comment on line 18: %{name}-%{shortcommit}.tar.xz

+ package name is OK
+ latest version
+ license is acceptable for Fedora (GPLv2+)
+ builds and installs OK
+ Provides/Requires/BuildRequires look OK (with Requires:rubygem(jekyll))
+ fedora-review finds no issues

Package is APPROVED.

Note: it is possible to make the binary package noarch by creating a subpackage and making it BuildArch:noarch and moving all the files to it. I'll attach a patch, but I'm not sure if this is worth the trouble — use it or ignore it as you see fit.

Created attachment 1338946 [details]
patch to make binary package noarch

(In reply to Zbigniew Jędrzejewski-Szmek from comment #19)
> It seems you actually need the dependency on jekyll, build fails otherwise.

It's weird but this is happening only in koji. I'd bet on old mock running on koji and not finding binaries installed via gem install. I've worked around it for now by detecting and using full path to the jekyll binary via:

%global jekyll_version 3.6.0
%global gems_dir %(gem environment | grep "USER INSTALLATION" | cut -d: -f2-)
....
%{gems_dir}/gems/jekyll-%{jekyll_version}/exe/jekyll

Thank you very much for the Review!

(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/fedora-developer-portal