Bug 1926697 - Review Request: openjdk-asmtools - An OpenSource project to develop tools for the production of proper and improper Java '.class' files
Summary: Review Request: openjdk-asmtools - An OpenSource project to develop tools for...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: jiri vanek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-02-09 09:52 UTC by Jaya
Modified: 2021-10-22 17:12 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-10-22 17:12:56 UTC
Type: ---
Embargoed:
jvanek: fedora-review+


Attachments (Terms of Use)

Description Jaya 2021-02-09 09:52:10 UTC
Spec URL: https://jhuttana.fedorapeople.org/asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/asmtools-7.0.b10.pre.0.1-pre.0.1.src.rpm
Description: AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
AsmTools consist of a set of (Java class file) assembler/disassemblers:
Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures, 
while providing Java VM specification compliant mnemonics for byte-code instructions.
JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
Fedora Account System Username:jhuttana

Comment 1 jiri vanek 2021-02-10 13:14:54 UTC
Please rename it to openjdk-asmtools.  asmtools is very generic and can point to nearly  anything.

Comment 2 Jaya 2021-02-10 16:59:45 UTC
I have made changes to spec file also to reflect the suggested change:
https://jhuttana.fedorapeople.org/

Comment 3 jiri vanek 2021-02-10 17:47:03 UTC
Please adapt the links then., Otherwise automation can not dinf them :)

Comment 5 jiri vanek 2021-02-10 17:50:44 UTC
nvm, done on your behalf. On quick glance all looks good. Running fedora-review now.

Comment 6 jiri vanek 2021-02-10 18:10:42 UTC
Please fix all [!] items and all rpmlint Errors, and some warings (eg the dot, but eg spell chekc is wrong and you are right)
Maybe except  "openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE" I'm not sure what it means, looks good to me.



Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Dist tag is present.


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU General Public License, Version
     2", "GNU General Public License v2.0 only". 5 files have unknown
     license. Detailed output of licensecheck in
     /home/jvanek/1926697-openjdk-asmtools/licensecheck.txt

Afaik this is GPLv2 not LGPLv2

[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.

Please add version 

[ ]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary.

You miss Requires java-headless. Please verify that jre is enough, and no jdk is needed.

[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0: https://github.com/openjdk/openjdk-
     asmtools/archive/openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
     See: https://docs.fedoraproject.org/en-US/packaging-
     guidelines/SourceURL/

This is ok as there is no upstream release with maven bindings yet

[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[!]: Final provides and requires are sane (see attachments).

See above

[!]: Package functions as described.

Please add launcher(s)

openjdk-asmtools launcher is a must.  openjdk-asmtools-{jasm,jdis,jcoder,jdec,jcdec} would be nice to have :)


[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[!]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define major 7.0, %define
     minor b09

Please try to fix this

[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: SourceX is a working URL.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openjdk-asmtools-7.0.b10.pre.0.1-pre.0.1.noarch.rpm
          openjdk-asmtools-7.0.b10.pre.0.1-pre.0.1.src.rpm
openjdk-asmtools.noarch: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.noarch: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.noarch: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.noarch: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.noarch: W: no-version-in-last-changelog
openjdk-asmtools.noarch: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
openjdk-asmtools.src: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.src: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.src: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.src: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.src: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.src: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.src: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.src: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.src: W: no-version-in-last-changelog
openjdk-asmtools.src: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.src: W: inconsistent-file-extension openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
openjdk-asmtools.src:15: W: macro-in-comment %{name}
openjdk-asmtools.src:15: W: macro-in-comment %{major}
openjdk-asmtools.src:15: W: macro-in-comment %{minor}
openjdk-asmtools.src:35: W: macro-in-comment %setup
openjdk-asmtools.src:35: W: macro-in-comment %{version}
openjdk-asmtools.src: E: specfile-error warning: Macro expanded in comment on line 15: %{name}/archive/%{major}-%{minor}.tar.gz
openjdk-asmtools.src: E: specfile-error 
openjdk-asmtools.src: E: specfile-error warning: Macro expanded in comment on line 15: %{name}/archive/%{major}-%{minor}.tar.gz
2 packages and 0 specfiles checked; 18 errors, 18 warnings.




Rpmlint (installed packages)
----------------------------
openjdk-asmtools.noarch: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.noarch: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.noarch: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.noarch: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.noarch: W: no-version-in-last-changelog
openjdk-asmtools.noarch: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
1 packages and 0 specfiles checked; 8 errors, 6 warnings.



Requires
--------
openjdk-asmtools (rpmlib, GLIBC filtered):



Provides
--------
openjdk-asmtools:
    openjdk-asmtools



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1926697
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Generic
Disabled plugins: C/C++, Python, Haskell, R, SugarActivity, Perl, PHP, Ocaml, Java, fonts
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 7 jiri vanek 2021-02-10 18:11:58 UTC
Please fix all [!] items and all rpmlint Errors, and some warings (eg the dot, but eg spell chekc is wrong and you are right)
Maybe except  "openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE" I'm not sure what it means, looks good to me.



Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Dist tag is present.


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU General Public License, Version
     2", "GNU General Public License v2.0 only". 5 files have unknown
     license. Detailed output of licensecheck in
     /home/jvanek/1926697-openjdk-asmtools/licensecheck.txt

Afaik this is GPLv2 not LGPLv2

[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.

Please add version 

[ ]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary.

You miss Requires java-headless. Please verify that jre is enough, and no jdk is needed.

[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0: https://github.com/openjdk/openjdk-
     asmtools/archive/openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
     See: https://docs.fedoraproject.org/en-US/packaging-
     guidelines/SourceURL/

This is ok as there is no upstream release with maven bindings yet

[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[!]: Final provides and requires are sane (see attachments).

See above

[!]: Package functions as described.

Please add launcher(s)

openjdk-asmtools launcher is a must.  openjdk-asmtools-{jasm,jdis,jcoder,jdec,jcdec} would be nice to have :)


[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[!]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define major 7.0, %define
     minor b09

Please try to fix this

[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: SourceX is a working URL.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openjdk-asmtools-7.0.b10.pre.0.1-pre.0.1.noarch.rpm
          openjdk-asmtools-7.0.b10.pre.0.1-pre.0.1.src.rpm
openjdk-asmtools.noarch: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.noarch: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.noarch: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.noarch: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.noarch: W: no-version-in-last-changelog
openjdk-asmtools.noarch: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
openjdk-asmtools.src: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.src: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.src: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.src: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.src: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.src: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.src: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.src: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.src: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.src: W: no-version-in-last-changelog
openjdk-asmtools.src: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.src: W: inconsistent-file-extension openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
openjdk-asmtools.src:15: W: macro-in-comment %{name}
openjdk-asmtools.src:15: W: macro-in-comment %{major}
openjdk-asmtools.src:15: W: macro-in-comment %{minor}
openjdk-asmtools.src:35: W: macro-in-comment %setup
openjdk-asmtools.src:35: W: macro-in-comment %{version}
openjdk-asmtools.src: E: specfile-error warning: Macro expanded in comment on line 15: %{name}/archive/%{major}-%{minor}.tar.gz
openjdk-asmtools.src: E: specfile-error 
openjdk-asmtools.src: E: specfile-error warning: Macro expanded in comment on line 15: %{name}/archive/%{major}-%{minor}.tar.gz
2 packages and 0 specfiles checked; 18 errors, 18 warnings.




Rpmlint (installed packages)
----------------------------
openjdk-asmtools.noarch: W: summary-ended-with-dot C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: summary-too-long C An OpenSource project to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: W: spelling-error %description -l en_US disassemblers -> disassembles, dis assemblers, dis-assemblers
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools OpenSource project which helps to develop tools for the production of proper and improper Java '.class' files.
openjdk-asmtools.noarch: E: description-line-too-long C Also, aids a community of Java .class file production for various testing and other OpenJDK development applications.
openjdk-asmtools.noarch: E: description-line-too-long C Jasm/Jdis: an assembler language that provides a Java-like declaration of member signatures,
openjdk-asmtools.noarch: E: description-line-too-long C while providing Java VM specification compliant mnemonics for byte-code instructions.
openjdk-asmtools.noarch: E: description-line-too-long C JCod/JDec: an assembler language that provides byte-code containers of class-file constructs.
openjdk-asmtools.noarch: E: description-line-too-long C AsmTools are developed to support the latest class file formats, in lock-step with JDK development.
openjdk-asmtools.noarch: W: no-version-in-last-changelog
openjdk-asmtools.noarch: W: invalid-url URL: https://github.com/openjdk/openjdk-asmtools HTTP Error 404: Not Found
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
1 packages and 0 specfiles checked; 8 errors, 6 warnings.



Requires
--------
openjdk-asmtools (rpmlib, GLIBC filtered):



Provides
--------
openjdk-asmtools:
    openjdk-asmtools



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1926697
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Generic
Disabled plugins: C/C++, Python, Haskell, R, SugarActivity, Perl, PHP, Ocaml, Java, fonts
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 8 Jaya 2021-02-25 10:38:11 UTC
I have uploaded the below after addressing the review comments provided above:

Spec URL: https://jhuttana.fedorapeople.org/openjdk-asmtools
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10.pre.0.1-1.fc35.src.rpm
Launcher Script URL: https://jhuttana.fedorapeople.org/openjdk-asmtools

Could you please review and let me know the comments?

Comment 9 jiri vanek 2021-02-25 11:53:14 UTC
hi!

The link to specfile is broken, but NVM, was easy to find.
thanx for update, the specfile and launcher went  a bit wild, before continuinfg with review, many nits:

BuildRequires:  java-1.8.0-openjdk
 -> This is wrong. Should be just java-devel. Please consult java packagig guidelines


sed -i "s|ln -sv|cp -r|g" mvngen.sh
 -> Good idea! However, why "cp -r?" Are there really some directories linked?


%files
%license LICENSE
%doc README.md
/usr/share/java/openjdk-asmtools/*
/usr/share/javadoc/openjdk-asmtools/*
/usr/share/maven-metadata/*
/usr/share/maven-poms/openjdk-asmtools/*
%attr(755, root, -) %{_bindir}/openjdk-asmtools

-> I really od not like aserixes here. Please enumerate what can be enumerated
-> You should not belistin here anything except the last one andmaybe docs and license. All others *have to be * gatehered by mvn_isntall and thus used via "-f .mfiles"



The launcher is complete mess :)) Please remove everythinf redundant.
Not, that usage of export _prefer_jre=false is superwrong.
-Djdk.attach.allowAttachSelf=true used without justification is secuirty hole.
Minimal:

#!/usr/bin/bash
set -e
# JPackage Project <http://www.jpackage.org/>
# Source functions library
. /usr/share/java-utils/java-functions
MAIN_CLASS=org.openjdk.asmtools.Main
_set_java_home
set_jvm
set_classpath openjdk-asmtools
run   "$@"

Should do. But I had not tried.



Please. for %{_bindir}/openjdk-asmtools add and isntall manpage. It is simple. Use output from %{_bindir}/openjdk-asmtools -h . Again, you can inspire in java-runtime-decompiler.


But looks good. Nice job! Looking forward for this pacakge.

Comment 10 Jaya 2021-03-01 07:25:02 UTC
sed -i "s|ln -sv|cp -r|g" mvngen.sh
 -> Good idea! However, why "cp -r?" Are there really some directories linked?

Yes. Because, if "-r "  is not used then , "sh mvngen.sh" fails as below:
$ sh mvngen.sh
Generating ~/asmtools/maven/pom.xml for asmtools 7.0 ea 10 (Java Assembler Tools)
Done
Creating symlinks to symulate maven structure
mkdir: created directory '~/asmtools/maven/src'
mkdir: created directory '~/asmtools/maven/src/main'
mkdir: created directory '~/asmtools/maven/src/main/java'
~/asmtools/maven/src/main/java
cp: -r not specified; omitting directory '../../../../src/org/'

Comment 11 Jaya 2021-03-01 07:29:08 UTC
%files
%license LICENSE
%doc README.md
/usr/share/java/openjdk-asmtools/*
/usr/share/javadoc/openjdk-asmtools/*
/usr/share/maven-metadata/*
/usr/share/maven-poms/openjdk-asmtools/*
%attr(755, root, -) %{_bindir}/openjdk-asmtools

-> I really od not like aserixes here. Please enumerate what can be enumerated
-> You should not belistin here anything except the last one andmaybe docs and license. All others *have to be * gatehered by mvn_isntall and thus used via "-f .mfiles"

Usage of .mfiles in the spec leads to :

Processing files: openjdk-asmtools-7.0.b10.pre.0.1-1.fc33.noarch
error: Could not open %files file ~/rpmbuild/BUILD/asmtools-master/.mfiles: No such file or directory

Spec file change:
' ' '
%install
rm -rf $RPM_BUILD_ROOT
mkdir -p $RPM_BUILD_ROOT%{_bindir}/
pushd maven
install -m 755 %{SOURCE1} $RPM_BUILD_ROOT%{_bindir}/
%mvn_install

%files -f .mfiles
%license LICENSE
%doc README.md
%attr(755, root, -) %{_bindir}/openjdk-asmtools
' ' '

So far I am not getting what is problem here!
Do you see anything obvious mistake here?

Comment 12 Fabio Valentini 2021-03-01 08:14:31 UTC
Use:
%files -f maven/.mfiles

The .mfiles file is generated in the directory the %mvn_* macros are run in.

Your pushd call is also missing a corresponding popd call.

Comment 13 Jaya 2021-03-01 10:51:31 UTC
@decathorpe 
Thanks for your suggestion to use : %files -f maven/.mfiles

Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10.pre.0.1-1.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher URL: https://jhuttana.fedorapeople.org/openjdk-asmtools

But for 'javadoc' I was getting error :
RPM build errors:
    Found bdb Packages database while attempting sqlite backend: using bdb backend.
    Installed (but unpackaged) file(s) found:
   /usr/share/javadoc/openjdk-asmtools/*


So, I retained "/usr/share/javadoc/openjdk-asmtools/*" under "%files -f maven/.mfiles"

spec:
' ' '
%files -f maven/.mfiles
%license LICENSE
%doc README.md
/usr/share/javadoc/openjdk-asmtools/*
%attr(755, root, -) %{_bindir}/openjdk-asmtools
%{_mandir}/man1/openjdk-asmtools.1*
' ' '

Comment 15 Fabio Valentini 2021-03-01 12:11:34 UTC
Yes, this is like the recommended approach for packaging maven projects:
https://fedora-java.github.io/howto/latest/#maven

However, I see one big issue: do **never** reference an upstream *branch*, only *commits* or *tags*.
Branches are moving targets, so it is unclear which state of the repository you are referencing.

Please refer to https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_hosting_services
for the latest Guidelines on how to handle git snapshots (there's even ready-for-use snippets for GitHub).

Comment 16 Fabio Valentini 2021-03-01 12:14:43 UTC
There seems to be another problem: Your Source0 URL references %{name}-%{version}.tar.gz, but the %setup call does still use "master".
That most certainly won't work.

You also don't need to document a manual wget call to download sources:
"spectool -g path-to.spec" will download all Sources and Patches for you.

Comment 17 jiri vanek 2021-03-01 12:40:53 UTC
Yo still have "# java runtime decompiler launch script" in the launcher :)

But nvm, will go through that more deeply.

Comment 18 Jaya 2021-03-01 13:22:27 UTC
(In reply to jiri vanek from comment #17)
> Yo still have "# java runtime decompiler launch script" in the launcher :)
> 
> But nvm, will go through that more deeply.

Yes :) my bad !! somehow missed that ;)
Now I have modified that comment line.

Comment 19 jiri vanek 2021-03-01 16:21:09 UTC
%global major 7.0
%global minor b09

Name:           openjdk-asmtools
Version:        7.0.b10.pre.0.1

I think it should be

%global major 7.0
%global minor b10

Name:           openjdk-asmtools
Version:        %{major}.%{minor}.pre.0.1

or not?

#%setup -q -n asmtools-%{version}

should be
#%%setup -q -n asmtools-%%{version}
Setup is multiline macro.

Ohterwise it looks really good.
I have one more "strange" request. which you mal revoke, but woudl be nice to have.
Can you please change https://jhuttana.fedorapeople.org/openjdk-asmtools to  openjdk-asmtools.in and generate from it openjdk-asmtools, openjdk-asmtools-jasm, openjdk-asmtools-jdis, openjdk-asmtools-jcoder, openjdk-asmtools-jdec and  openjdk-asmtools-jcdec  ? the onl diffeerence will be in main method an name... A bit of absh exercise :)

Manual testing is now pass from me. Will push the review again.

Comment 20 jiri vanek 2021-03-01 16:27:35 UTC
Man page for: openjdk-asmtools is good. TY
Man pages for openjdk-asmtools-jasm, openjdk-asmtools-jdis, openjdk-asmtools-jcoder, openjdk-asmtools-jdec and  openjdk-asmtools-jcdec are NOT necessary

Comment 21 jiri vanek 2021-03-01 16:48:54 UTC
[!] missing Requires java-headless. Please doule check that JRE is really the only needed thing and nothing from SDK is used.
[!] bad version in changelog
[!] the javadoc package was not generated
[!] please fix the warnings and errors of rpmlint. Some of them are nto valid (eg spellchekckers) but most are. Indeed. I'm really not sure about E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE ; afaik it is corrrect, and it isa %license provided
[!] Fabio is right in c#15 - please instead of wget -O asmtools-7.0.b10.pre.0.1.tar.gz https://github.com/openjdk/asmtools/archive/master.zip use snapshot of current latest commit in master. Issue is, that if one would attmept to regenerate the sources, from yor link they wouldbe different. From exact commit, they will be same.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- This seems like a Java package, please install fedora-review-plugin-java
  to get additional checks


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU General Public License, Version
     2", "GNU General Public License v2.0 only". 5 files have unknown
     license. Detailed output of licensecheck in
     /home/jvanek/1926697-openjdk-asmtools/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: Package consistently uses macros (instead of hard-coded directory
     names). (see #19)
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[!]: Requires correct, justified where necessary - Mising java-headless
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build

===== SHOULD items =====

Generic:
[x]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0: https://github.com/openjdk/openjdk-
     asmtools/archive/openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
     See: https://docs.fedoraproject.org/en-US/packaging-
     guidelines/SourceURL/
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[!]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[-]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openjdk-asmtools-7.0.b10.pre.0.1-1.fc34.noarch.rpm
          openjdk-asmtools-javadoc-7.0.b10.pre.0.1-1.fc34.noarch.rpm
          openjdk-asmtools-7.0.b10.pre.0.1-1.fc34.src.rpm
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: W: incoherent-version-in-changelog 0.1-1 ['7.0.b10.pre.0.1-1.fc34', '7.0.b10.pre.0.1-1']
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
openjdk-asmtools.src: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.src: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.src: W: inconsistent-file-extension openjdk-asmtools-7.0.b10.pre.0.1.tar.gz
openjdk-asmtools.src:47: W: macro-in-comment %setup
openjdk-asmtools.src:47: W: macro-in-comment %{version}
openjdk-asmtools.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 5, tab: line 18)
openjdk-asmtools.src: W: invalid-url Source0: https://github.com/openjdk/openjdk-asmtools/archive/openjdk-asmtools-7.0.b10.pre.0.1.tar.gz HTTP Error 404: Not Found
3 packages and 0 specfiles checked; 1 errors, 10 warnings.




Rpmlint (installed packages)
----------------------------
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: W: incoherent-version-in-changelog 0.1-1 ['7.0.b10.pre.0.1-1.fc34', '7.0.b10.pre.0.1-1']
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
2 packages and 0 specfiles checked; 1 errors, 3 warnings.



Requires
--------
openjdk-asmtools (rpmlib, GLIBC filtered):
    /usr/bin/bash
    java-headless
    javapackages-filesystem

openjdk-asmtools-javadoc (rpmlib, GLIBC filtered):
    javapackages-filesystem



Provides
--------
openjdk-asmtools:
    mvn(org.openjdk:asmtools)
    mvn(org.openjdk:asmtools:pom:)
    openjdk-asmtools

openjdk-asmtools-javadoc:
    openjdk-asmtools-javadoc



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1926697
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Java, Shell-api
Disabled plugins: PHP, Haskell, fonts, R, Python, Perl, SugarActivity, C/C++, Ocaml
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 22 jiri vanek 2021-03-01 16:50:14 UTC
> [!] missing Requires java-headless. Please doule check that JRE is really the only needed thing and nothing from SDK is used.

java-headless is generated requirement. Pls verify it is enough. Nothng from SDK (java-devel) should be needed.

Comment 23 Fabio Valentini 2021-03-01 17:26:43 UTC
I wonder where the .pre.0.1 part of the Version comes from?
It does not exist in the upstream project.

If you want to denote that it's a pre-release snapshot, then you're doing it wrong, since your string sorts higher than the eventual Release string. Instead, you should use:
Release: 0.1.{snapshot info}%{?dist}

See: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

Comment 24 Jaya 2021-03-02 05:40:24 UTC
(In reply to jiri vanek from comment #19)
> %global major 7.0
> %global minor b09
> 
> Name:           openjdk-asmtools
> Version:        7.0.b10.pre.0.1
> 
> I think it should be
> 
> %global major 7.0
> %global minor b10
> 
> Name:           openjdk-asmtools
> Version:        %{major}.%{minor}.pre.0.1
> 
> or not?
> 
> #%setup -q -n asmtools-%{version}
> 
> should be
> #%%setup -q -n asmtools-%%{version}
> Setup is multiline macro.
> 

I will make these changes

> Ohterwise it looks really good.
> I have one more "strange" request. which you mal revoke, but woudl be nice
> to have.
> Can you please change https://jhuttana.fedorapeople.org/openjdk-asmtools to 
> openjdk-asmtools.in and generate from it openjdk-asmtools,
> openjdk-asmtools-jasm, openjdk-asmtools-jdis, openjdk-asmtools-jcoder,
> openjdk-asmtools-jdec and  openjdk-asmtools-jcdec  ? the onl diffeerence
> will be in main method an name... A bit of absh exercise :)
> 
I will take up this at the end :)
> Manual testing is now pass from me. Will push the review again.

Comment 25 Jaya 2021-03-02 05:43:23 UTC
(In reply to Fabio Valentini from comment #23)
> I wonder where the .pre.0.1 part of the Version comes from?
> It does not exist in the upstream project.
> 
> If you want to denote that it's a pre-release snapshot, then you're doing it
> wrong, since your string sorts higher than the eventual Release string.
> Instead, you should use:
> Release: 0.1.{snapshot info}%{?dist}
> 
> See:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots

Before also I went through this pointer but I was not getting it :(
I will try to make the changes and upload then may be in the review I will get to know whether I understood it right or not ;)

Comment 26 Jaya 2021-03-02 06:24:57 UTC
I have addressed the latest review comments and uploaded the files

Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10.pre.0.1-0.1.20210122.7eadbf.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher URL: https://jhuttana.fedorapeople.org/openjdk-asmtools

(In reply to Fabio Valentini from comment #23)
> I wonder where the .pre.0.1 part of the Version comes from?
> It does not exist in the upstream project.
> 
> If you want to denote that it's a pre-release snapshot, then you're doing it
> wrong, since your string sorts higher than the eventual Release string.
> Instead, you should use:
> Release: 0.1.{snapshot info}%{?dist}
> 
> See:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots

I tried to understand <snapinfo> and modified the 'Release' tag. Could you please take a look?

(In reply to jiri vanek from comment #21)
> [!] Fabio is right in c#15 - please instead of wget -O
> asmtools-7.0.b10.pre.0.1.tar.gz
> https://github.com/openjdk/asmtools/archive/master.zip use snapshot of
> current latest commit in master. Issue is, that if one would attmept to
> regenerate the sources, from yor link they wouldbe different. From exact
> commit, they will be same.
> 

Yes. I am not getting how to do that !!
I will try to find out.

And also, for the rpmlint warning:
> openjdk-asmtools.noarch: W: incoherent-version-in-changelog 0.1-1 ['7.0.b10.pre.0.1-1.fc34', '7.0.b10.pre.0.1-1'] , 
I have made the change in the changelog entry looking at the srpm generated : openjdk-asmtools-7.0.b10.pre.0.1-0.1.20210122.7eadbf.fc35.src.rpm

Comment 27 jiri vanek 2021-03-02 08:49:26 UTC
(In reply to Fabio Valentini from comment #23)
> I wonder where the .pre.0.1 part of the Version comes from?

To use this versioning was my suggestion, aas it is indeed pre of b10.

> It does not exist in the upstream project.

Asmtools is released quite seldom, after few commits always. We can expect b10 anytime.
We can not use b09, as it lacks the maven binding.
From the options to use b09 + all usptream patches until now, or to pick up master in altes commit, I recommended Jaya to pick up latest commit as sources.

> 
> If you want to denote that it's a pre-release snapshot, then you're doing it
> wrong, since your string sorts higher than the eventual Release string.
> Instead, you should use:
> Release: 0.1.{snapshot info}%{?dist}
> 
> See:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots

The only reason for fiddling with pre.0.1 is to ensure update when  non-pre is released.

So the
%global major 7.0
%global minor b10
Name:           openjdk-asmtools
Version:        %{major}.%{minor}
Release:        pre.0.1%{?dist}

Was afaik correct, as 
Version:        %{major}.%{minor}
Release:        1%{?dist}

Will always surpass it.


Current
Name:           openjdk-asmtools
Version:        %{major}.%{minor}.pre.0.1
Release:        0.1.20210122.7eadbf%{?dist}

Is afaik wrong. pre.0.1 do nto belong to version, but to release.  Where the  hashset belongs to version.  and am not sure with date. In all cases  the date (and maybe also the hash) in the packkage  can casue it not being updatable i future, by regular :
Version:        %{major}.%{minor}
Release:        1%{?dist}

I would like to highlight - the "pre.0.1" have major reason in get update by  "1" and that having hash or date  in rpm version can be dangerous.


Fabio, before continuing with the package review, can you state your final judgement, exactly what version you wish to see in the pkg?
I'm heavily in favour of original
%global major 7.0
%global minor b10
Name:           openjdk-asmtools
Version:        %{major}.%{minor}
Release:        pre.0.1%{?dist}

With fixed sources (not in latest spec) being clearly named with commit hash in both comment and tarball name. But not reflecting any date not hash in rpm version.

Comment 28 jiri vanek 2021-03-02 09:03:42 UTC
jaya, pls use:
https://github.com/openjdk/asmtools/archive/7eadbbf8f41b8174f7991a5ad1fe79519fe108d4.tar.gz

As source. Feel free to use it as it is or rename it. But for sure use it in the comment.
Please do not rename the suffix. IN your curren tapproach you downlaod zip and renamed it to tar.gz!

Comment 29 jiri vanek 2021-03-02 09:06:40 UTC
jaya, the javadoc package works for you? Is it empty, does rpm with it even exists? 
it do nto work for me. If it do not work for you, then you hsould probably remove all javadoc subpackage declarations. But of course it is always better to generate it.

Comment 30 Fabio Valentini 2021-03-02 09:07:19 UTC
%global major 7.0
%global minor b10
Name:           openjdk-asmtools
Version:        %{major}.%{minor}
Release:        pre.0.1%{?dist}

- "pre" does not belong anywhere
- which upstream commit is used? it is not defined

Something like this should be used if you want to use "pre-release" snapshot versioning:

%global major 7.0
%global minor b10

%global commit 40-CHARACTER-HASH-VALUE
%global shortcommit %(c=%{commit}; echo ${c:0:7})
%global commitdate YYYYMMDD

Name:           openjdk-asmtools
Version:        %{major}.%{minor}
Release:        0.1.%{commitdate}.git%{shortcommit}%{?dist}
[...]
Source0:        https://github.com/openjdk/asmtools/archive/%{commit}/%{name}-%{shortcommit}.tar.gz
[...]
%autosetup -n asmtools-%{commit}

Or, if you want to use "post-snapshot" versioning instead, it should be:

%global major 7.0
%global minor b09

%global commit 40-CHARACTER-HASH-VALUE
%global shortcommit %(c=%{commit}; echo ${c:0:7})
%global commitdate YYYYMMDD

Name:           openjdk-asmtools
Version:        %{major}.%{minor}
Release:        1.%{commitdate}.git%{shortcommit}%{?dist}
[...]
Source0:        https://github.com/openjdk/asmtools/archive/%{commit}/%{name}-%{shortcommit}.tar.gz
[...]
%autosetup -n asmtools-%{commit}


The only differences are:

- leading "0." in Release for pre-release snapshot, such that proper version sorts higher
- no leading "0." for post-release snapshot, such that it will sort higher than last proper version

But specifying the commit and date is mandatory in either case.

(I agree, the <snapinfo> stuff from the Guidelines is very hard to understand.)

And do not download sources manually. If necessary, use "spectool -g", it will evaluate macros from the spec use the correct output name.

Comment 31 Jaya 2021-03-02 09:48:08 UTC
(In reply to jiri vanek from comment #29)
> jaya, the javadoc package works for you? Is it empty, does rpm with it even
> exists? 
> it do nto work for me. If it do not work for you, then you hsould probably
> remove all javadoc subpackage declarations. But of course it is always
> better to generate it.

javadoc related rpm is generated for me. I can see the below in my build output:

' ' '
Checking for unpackaged file(s): /usr/lib/rpm/check-files ~/rpmbuild/BUILDROOT/openjdk-asmtools-7.0.b10-pre.0.1.fc33.x86_64
Wrote: ~/rpmbuild/RPMS/noarch/openjdk-asmtools-7.0.b10-pre.0.1.fc33.noarch.rpm
Wrote: ~/rpmbuild/RPMS/noarch/openjdk-asmtools-javadoc-7.0.b10-pre.0.1.fc33.noarch.rpm
' ' '

Comment 32 Jaya 2021-03-02 10:17:18 UTC
(In reply to Fabio Valentini from comment #30)
> %global major 7.0
> %global minor b10
> Name:           openjdk-asmtools
> Version:        %{major}.%{minor}
> Release:        pre.0.1%{?dist}
> 
> - "pre" does not belong anywhere
> - which upstream commit is used? it is not defined
> 
> Something like this should be used if you want to use "pre-release" snapshot
> versioning:
> 
> %global major 7.0
> %global minor b10
> 
> %global commit 40-CHARACTER-HASH-VALUE
> %global shortcommit %(c=%{commit}; echo ${c:0:7})
> %global commitdate YYYYMMDD
> 
> Name:           openjdk-asmtools
> Version:        %{major}.%{minor}
> Release:        0.1.%{commitdate}.git%{shortcommit}%{?dist}
> [...]
> Source0:       
> https://github.com/openjdk/asmtools/archive/%{commit}/%{name}-%{shortcommit}.
> tar.gz
> [...]
> %autosetup -n asmtools-%{commit}
> 
> Or, if you want to use "post-snapshot" versioning instead, it should be:
> 
> %global major 7.0
> %global minor b09
> 
> %global commit 40-CHARACTER-HASH-VALUE
> %global shortcommit %(c=%{commit}; echo ${c:0:7})
> %global commitdate YYYYMMDD
> 
> Name:           openjdk-asmtools
> Version:        %{major}.%{minor}
> Release:        1.%{commitdate}.git%{shortcommit}%{?dist}
> [...]
> Source0:       
> https://github.com/openjdk/asmtools/archive/%{commit}/%{name}-%{shortcommit}.
> tar.gz
> [...]
> %autosetup -n asmtools-%{commit}
> 
> 
> The only differences are:
> 
> - leading "0." in Release for pre-release snapshot, such that proper version
> sorts higher
> - no leading "0." for post-release snapshot, such that it will sort higher
> than last proper version
> 
> But specifying the commit and date is mandatory in either case.
> 
> (I agree, the <snapinfo> stuff from the Guidelines is very hard to
> understand.)
> 
> And do not download sources manually. If necessary, use "spectool -g", it
> will evaluate macros from the spec use the correct output name.

Thank you Fabino!! Your suggestions were very handy for me :) I have included the suggested changes to spec file and uploaded.
@decathor and @jvane : Could you please take a look?

Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher URL: https://jhuttana.fedorapeople.org/openjdk-asmtools

Comment 33 jiri vanek 2021-03-02 12:35:10 UTC
@fabio
Thanx a lot!

@jaya
You still mess the requires x buildrequires
  BuildRequires:  java-headless 
is nonsense. Your build requires  java-devel , which is transitively inherited from buildrequiring of maven-local. Feel free to add BuildRequires:  java-devel, but drop BuildRequires:  java-headless

Your package have generated 
  Requires:  java-headless
Have you verified it runs on jre only?
If not, you have to 
  Requires:  java-devel

But I think it should work.

Comment 34 jiri vanek 2021-03-02 12:37:04 UTC
Please add the sub launcher is you can, as for https://bugzilla.redhat.com/show_bug.cgi?id=1926697#c20 and https://bugzilla.redhat.com/show_bug.cgi?id=1926697#c19 
That will cost you nothing and will be superuseful.

Otherwise it loosk already good.

Comment 35 Jaya 2021-03-02 13:40:33 UTC
(In reply to jiri vanek from comment #33)
> @fabio
> Thanx a lot!
> 
> @jaya
> You still mess the requires x buildrequires
>   BuildRequires:  java-headless 
> is nonsense. Your build requires  java-devel , which is transitively
> inherited from buildrequiring of maven-local. Feel free to add
> BuildRequires:  java-devel, but drop BuildRequires:  java-headless
> 

Sure , I have retained BuildRequires:  java-devel as it is needed for build.

> Your package have generated 
>   Requires:  java-headless
> Have you verified it runs on jre only?

Yes only JRE is needed to run this package.

> If not, you have to 
>   Requires:  java-devel
> 
> But I think it should work.

I have uploaded the changes:
Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher URL: https://jhuttana.fedorapeople.org/openjdk-asmtools

Comment 36 jiri vanek 2021-03-02 16:22:15 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- This seems like a Java package, please install fedora-review-plugin-java
  to get additional checks


===== MUST items =====

Generic:
[x ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU General Public License, Version
     2", "GNU General Public License v2.0 only". 5 files have unknown
     license. Detailed output of licensecheck in
     /home/jvanek/Desktop/todo2/1926697-openjdk-asmtools/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.noarch.rpm
          openjdk-asmtools-javadoc-7.0.b10-0.1.20210122.git7eadbbf.fc35.noarch.rpm
          openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.src.rpm
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
openjdk-asmtools.src: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.src: W: spelling-error %description -l en_US JCod -> J Cod, Cod
3 packages and 0 specfiles checked; 1 errors, 4 warnings.




Rpmlint (installed packages)
----------------------------
openjdk-asmtools.noarch: W: spelling-error %description -l en_US Jasm -> Jams, Jam, Jasmine
openjdk-asmtools.noarch: W: spelling-error %description -l en_US JCod -> J Cod, Cod
openjdk-asmtools.noarch: E: incorrect-fsf-address /usr/share/licenses/openjdk-asmtools/LICENSE
2 packages and 0 specfiles checked; 1 errors, 2 warnings.



Source checksums
----------------
https://github.com/openjdk/asmtools/archive/7eadbbf8f41b8174f7991a5ad1fe79519fe108d4/openjdk-asmtools-7eadbbf.tar.gz :
  CHECKSUM(SHA256) this package     : 5fdb6e98f3c23cbcd98e8b893a5f491e8c97281b58662045350ce84617286a91
  CHECKSUM(SHA256) upstream package : 5fdb6e98f3c23cbcd98e8b893a5f491e8c97281b58662045350ce84617286a91


Requires
--------
openjdk-asmtools (rpmlib, GLIBC filtered):
    /usr/bin/bash
    java-headless
    javapackages-filesystem

openjdk-asmtools-javadoc (rpmlib, GLIBC filtered):
    javapackages-filesystem



Provides
--------
openjdk-asmtools:
    mvn(org.openjdk:asmtools)
    mvn(org.openjdk:asmtools:pom:)
    openjdk-asmtools

openjdk-asmtools-javadoc:
    openjdk-asmtools-javadoc



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -b 1926697
Buildroot used: fedora-rawhide-x86_64
Active plugins: Java, Generic, Shell-api
Disabled plugins: SugarActivity, PHP, R, Python, Haskell, fonts, Perl, C/C++, Ocaml
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Package is approved.

Lokng forward for sub-launchers

Comment 37 Jaya 2021-03-03 06:51:31 UTC
Added sub-launchers. Could you please verify?

I have uploaded the changes:
Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher.in URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.in

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools
Usage: 
to run an assembly tool: 
   $ java -jar asmtools.jar toolName [args...] 
   where toolName one of: jasm, jdis, jcoder, jdec, jcdec 
to get the version: 
   $ java -jar asmtools.jar -version 
to get this message 
   $ java -jar asmtools.jar -?|-h|-help

No arguments provided!  See options above.

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools-jasm
Usage: java -jar asmtools.jar jasm [options] file.jasm...
where possible options include:
     -d destdir  directory to place resulting .class files
     -g          add debug information
     -v          add trace information
     -nowrite    do not write resulting .class files
     -nowarn     do not print warnings
     -strict     consider warnings as errors
     -cv major.minor  set operating class file version (by default 45.3)
     -version    prints the program version

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools-jcdec
Usage: java -jar asmtools.jar jcdec [options] FILE.class... > FILE.jcod
where possible options include:
     -g:  detailed output format
     -version:  print version number and date

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools-jcoder
Usage: java -jar asmtools.jar jcoder [options] file.jcod...
where possible options include:
     -nowrite     do not write resulting .class files
     -ignore      ingore non-fatal error(s) that suppress writing .class files
     -d destdir   directory to place resulting .class files
     -version     prints the program version

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools-jdec
Usage: java -jar asmtools.jar jdec [options] FILE.class... > FILE.jcod
where possible options include:
     -g:  detailed output format
     -version:  print version number and date

<mock-chroot> sh-5.0# /usr/bin/openjdk-asmtools-jdis
Usage: java -jar asmtools.jar jdis [options] FILE.class... > FILE.jasm
where possible options include:
     -g   detailed output format
     -sl  source lines in comments
     -hx  hexadecimal numbers
     -v   debug mode
     -version   prints the version info

Comment 38 jiri vanek 2021-03-03 18:39:58 UTC
Hi!

you can simplify it a lot:
https://jvanek.fedorapeople.org/jaya/openjdk-asmtools.spec
https://jvanek.fedorapeople.org/jaya/openjdk-asmtools.in

Namely just added placeholder to the in file:
run @SCD@  "$@"

and simp;ifed loop to get rid of "if":
for launcher in "" "-jasm" "-jdis" "-jcoder" "-jdec" "-jcdec"; do
  switch=`echo $launcher |sed "s/-//"`
  cat %{SOURCE1} | sed "s/@SCD@/$switch/"  > $RPM_BUILD_ROOT%{_bindir}/%{name}$launcher
done

A must is to NOT mix spaces and tabs please.

Comment 39 Jaya 2021-03-04 05:31:26 UTC
Thank you Jiri!! That highly simplified version :)
Now the changes are done and verified.

Spec file URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.spec
SRPM URL: https://jhuttana.fedorapeople.org/openjdk-asmtools-7.0.b10-0.1.20210122.git7eadbbf.fc35.src.rpm
Man Page URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.1
Launcher.in URL: https://jhuttana.fedorapeople.org/openjdk-asmtools.in

Comment 40 jiri vanek 2021-03-04 09:21:36 UTC
Thanx. The package is already approved,nothng changed in meantime.
Please proceed with release.

Comment 41 Jaya 2021-03-04 09:54:55 UTC
Sure :) Thank you!

Comment 42 Tomas Hrcka 2021-03-04 11:41:41 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/openjdk-asmtools

Comment 43 Didik Supriadi 2021-10-22 17:12:56 UTC
This package is now in Fedora repo hence closing this review.


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