Bug 1542522 (jsonnet) - Review Request: jsonnet - a data templating language
Summary: Review Request: jsonnet - a data templating language
Keywords:
Status: CLOSED WONTFIX
Alias: jsonnet
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: William Moreno
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/google/jsonnet
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2018-02-06 13:56 UTC by Naadir Jeewa
Modified: 2019-11-13 06:58 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-11-13 06:57:35 UTC
Type: ---
Embargoed:
williamjmorenor: fedora-review?


Attachments (Terms of Use)

Description Naadir Jeewa 2018-02-06 13:56:46 UTC
Spec URL: https://raw.githubusercontent.com/randomvariable/jsonnet-rpm/master/jsonnet.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/randomvariable/jsonnet/srpm-builds/00710524/jsonnet-0.9.5-2.src.rpm
Description: A data templating language compiler.
From the website:

"Jsonnet is a domain specific configuration language that helps you define JSON data. Jsonnet lets you compute fragments of JSON within the structure, bringing the same benefit to structured data that templating languages bring to plain text."

Of growing interest in the configuration management arena, hence being submitted.
Fedora Account System Username:randomvariable

Comment 1 Artur Frenszek-Iwicki 2018-02-09 12:27:29 UTC
>Group: Development/Languages
The Group: tag is not used in Fedora.
https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

>License: Apache-2.0
The correct identifier for the Apache Software Licence v.2.0 in Fedora is "ASL 2.0".
https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses

>%package devel
As a general rule, devel packages should have a "Requires: %{name}%{?_isa} = %{version}-%{release}"; this ensures that you can't install devel headers for a different version of the library than the .so files provide.

>%package lib
Someone correct me if I'm wrong, but I think the preferred way is to have "libSOMETHING", like "%package -n libjsonnet". The development headers would then become "%package -n libjsonnet-devel".

>%if 0%{?fedora} >= 21
>%package python3
Seeing how Fedora 25 has been put to its grave almost two months ago, this conditional can be removed.

>%{__make}
It is preferred to use non-macro forms of system executables.
https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

You should also include the licence text in the packages, by putting the following in %files: "%license LICENSE"
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

Comment 3 Artur Frenszek-Iwicki 2018-02-13 10:52:21 UTC
>%if 0%{?rhel}
>Group: Development/Languages
>%endif
Sorry, I should have been more clear. Drop this completely. The Group: tag is not used in EPEL, either.

>License: Apache-2.0
Still not fixed. See my content above.

>install -m 0555 -s ./jsonnet $RPM_BUILD_ROOT%{_bindir}
>install -m 0444 ./include/* $RPM_BUILD_ROOT%{_includedir}
>install -m 0444 -s ./lib%{name}.so $RPM_BUILD_ROOT%{_libdir}
The default is to make all things writable by root, so this should probably be 0755 and 0644 (unless you have a good reason). Also, looking at /usr/lib on my system - libraries are usually installed with 755, not 644.

Regarding the licence: you should make sure that the licence text is present if the user installs any combination of packages. Currently the licence is present only in the main package. The lib package can be installed independently of the main package, so it should contain a copy of the licence text. The -devel and -python packages Require the lib package, so they don't need their own copy.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing

Comment 4 Naadir Jeewa 2018-02-13 11:38:13 UTC
> Sorry, I should have been more clear. Drop this completely. The Group: tag is not used in EPEL, either.

I wasn't sure from the docs, so took a guess.

>>License: Apache-2.0
> Still not fixed. See my content above.

Sorry, missed that one.

> The default is to make all things writable by root, so this should probably be
> 0755 and 0644 (unless you have a good reason). Also, looking at /usr/lib on my
> system - libraries are usually installed with 755, not 644.

Amended

> make sure that the licence text is present if the user installs any combination of packages

Amended
https://copr-be.cloud.fedoraproject.org/results/randomvariable/jsonnet/fedora-rawhide-x86_64/00714576-jsonnet/jsonnet.spec

https://copr-be.cloud.fedoraproject.org/results/randomvariable/jsonnet/fedora-rawhide-x86_64/00714576-jsonnet/jsonnet-0.9.5-4.src.rpm

Thanks for your time on this so far.

Naadir

Comment 6 William Moreno 2018-02-18 00:50:26 UTC
Just a quit "eye ball" checking:

The main package jsonnet must Requires: libjsonnet, 

In the other side you not following python packaging names conventios names are:

python2-jsonnet
python3-jsonet

Do we realley need a devel subpackage?

Comment 7 Itamar Reis Peixoto 2018-02-18 13:05:44 UTC
you don't need that 

%if 0%{?rhel}


just use BuildRequires: python2-setuptools and it will work on rhel too.

Comment 8 Itamar Reis Peixoto 2018-02-18 13:07:17 UTC
instead of 

BuildRequires: python3-devel

use

python%{python3_pkgversion}-devel

Comment 9 Itamar Reis Peixoto 2018-02-18 13:09:51 UTC
instead of


%if 0%{?fedora}
%files -n lib%{name}-python3
%{python3_sitearch}/*
%endif

use


%files -n lib%{name}-python%{python3_pkgversion}
%{python3_sitearch}/*


instead of

%package -n lib%{name}-python3

use
%package -n lib%{name}-python%{python3_pkgversion}

Comment 10 William Moreno 2018-03-16 16:56:30 UTC
Any update here? If not I will need to close this ticket as a DEADREVIEW

Comment 11 William Moreno 2018-06-26 16:26:15 UTC
Final ping


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