[Bug 1422555] New: Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

            Bug ID: 1422555
           Summary: Review Request: arduino-ctags - A mix of ctags and
                    anjuta-tags for the perfect C++ ctags
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: nobody(a)fedoraproject.org
          Reporter: giallu(a)gmail.com
        QA Contact: extras-qa(a)fedoraproject.org
                CC: package-review(a)lists.fedoraproject.org



Spec URL: https://giallu.fedorapeople.org/arduino-ctags.spec
SRPM URL:
https://giallu.fedorapeople.org/arduino-ctags-5.8-1.arduino11.fc24.src.rpm
Description: An Arduino fork of exuberant ctags, used by the Arduino IDE when
compilig sketches.
Fedora Account System Username: giallu

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Gianluca Sforna <giallu(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1316315




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1316315
[Bug 1316315] arduino-1.8.1 is available
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |randy(a)electronsweatshop.com
           Assignee|nobody(a)fedoraproject.org    |randy(a)electronsweatshop.com
              Flags|                            |fedora-review?



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |giallu(a)gmail.com
              Flags|                            |needinfo?(giallu(a)gmail.com)



--- Comment #1 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
There are a few things we must fix before this package can be approved:

[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/arduino/tools-
     builder/ctags/5.8arduino11, /usr/share/arduino, /usr/share/arduino
     /tools-builder/ctags, /usr/share/arduino/tools-builder

     Does it make sense for this package to own tools-builder? If
     not, should we request that arduino-core provide it?


[!]: Development files must be in a -devel package

     You should put the header files into a devel package.


[!]: Package obeys FHS, except libexecdir and /usr/target.

     The shared lib is arch-dependent, and so should go into
     %{_libdir}. Is make install not suffiencent for this package?


One optional suggestion from rpmlint:

arduino-ctags.x86_64: E: incorrect-fsf-address
/usr/share/licenses/arduino-ctags/COPYING

You could work with upstream to correct the fsf address in the COPYING file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Gianluca Sforna <giallu(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(giallu(a)gmail.com) |



--- Comment #2 from Gianluca Sforna <giallu(a)gmail.com&gt; ---
Thanks Randy for picking this up!

(In reply to Randy Barlow from comment #1)
>  There are a few things we must fix before this package can be
> approved:
>  
>  [!]: Package must own all directories that it creates.
>       Note: Directories without known owners: /usr/share/arduino/tools-
>       builder/ctags/5.8arduino11, /usr/share/arduino, /usr/share/arduino
>       /tools-builder/ctags, /usr/share/arduino/tools-builder
>  
>       Does it make sense for this package to own tools-builder? If
>       not, should we request that arduino-core provide it? 
I added the directory ownership here, since I am not sure we want to depend on
arduino-core; rather we will require this package from arduino-builder 


>  
>  
>  [!]: Development files must be in a -devel package
>       
>       You should put the header files into a devel package.
>  
>  
>  [!]: Package obeys FHS, except libexecdir and /usr/target.
>  
>       The shared lib is arch-dependent, and so should go into
>       %{_libdir}. Is make install not suffiencent for this package? 
I am not sure which library/headers you are referring to. For the purposes of
building arduino sketches I need only the ctags executable, and that has to be
placed in the specific directory I am using. This is the reason of the "weird"
install step.

>  
>  
>  One optional suggestion from rpmlint:
>  
>  arduino-ctags.x86_64: E: incorrect-fsf-address
>  /usr/share/licenses/arduino-ctags/COPYING
>  
>  You could work with upstream to correct the fsf address in the COPYING file. 
I will see if upstream can fix it; btw I noticed the ctags rpm package we have
in the repos has the same issue.


Spec URL: https://giallu.fedorapeople.org/arduino-ctags.spec
SRPM URL:
https://giallu.fedorapeople.org/arduino-ctags-5.8-2.arduino11.fc24.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Gianluca Sforna <giallu(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(randy@electronswe
                   |                            |atshop.com)



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(randy@electronswe |
                   |atshop.com)                 |



--- Comment #3 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
(In reply to Gianluca Sforna from comment #2)
>  I am not sure which library/headers you are referring to. For the
> purposes
>  of building arduino sketches I need only the ctags executable, and that has
>  to be placed in the specific directory I am using. This is the reason of the
>  "weird" install step. 
Hello Gianluca!

The library is the .so file:

$ file
rpms-unpacked/arduino-ctags-5.8-1.arduino11.fc26.x86_64.rpm/usr/share/arduino/tools-builder/ctags/5.8arduino11/ctags

rpms-unpacked/arduino-ctags-5.8-1.arduino11.fc26.x86_64.rpm/usr/share/arduino/tools-builder/ctags/5.8arduino11/ctags:
ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked,
interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32,
BuildID[sha1]=0f8f9fa4801c724f6a88c39ad3eb71cdd951b0cc, stripped, with
debug_info

That file is an x86_64 file on my machine, so it should not go into /usr/share.
Further, it appears to have a main() function - should it go into /usr/bin? If
not, it needs to go into %{_libdir}.

The header files are the .h files - typically packages will provide a -devel
subpackage that contains those, which is helpful for people who want to link
against your package. These files are usually installed under %{_includedir}.
For example, python2-devel installs the .h files in /usr/include/python2.7/ on
my system.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #4 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
Sorry - it's not an .so file by extension, but it is a shared object according
to file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #5 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
If that executable is not meant to be used by humans, it could also go into
%{_libexecdir}. It does seem to print out help text when run by itself, which
indicates to me that it might be intended for human use. If it's for humans, it
should go into %{_bindir}.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #6 from Gianluca Sforna <giallu(a)gmail.com&gt; ---
Apparently the "file" command returns the same info both for executables and
for shared libraries. However, this is indeed an executable, as you can verify
by running it with --help option.

Now, this executable is actually a fork of exuberant ctags (also in Fedora) so
I could not put it in %{_bindir} or I'd conflict with the ctags RPM. Besides,
even if it can be used by humans, the tool generates an index (or tag) file of
language objects found in source files that allows these items to be quickly
and easily located by a text editor or other utility; so the normal usage is by
means of other tools.
However, this particular fork is intended to be used _exclusively_ by Arduino;
think of this like a plugin or addon, as reflected by the namespacing (it's
arduino-ctags, not just ctags).

We could argue whether it could be a good idea to encourage the Arduino guys to
push their changes upstream (I think so) but right now I am just focusing on
having a working Arduino update with all the required tools in place.

Lastly, while I think it is possible to move the tool away from the current
installation directory, that means additional work to patch the main arduino
IDE package in order to let it find the new location. Since no other tool is
(and probably never will) using it, I can hardly justify the additional
complexity.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #7 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
I cannot approve this package if it puts arch-dependent files into /usr/share -
that's a violation of the FHS and thus a violation of Fedora's packaging
guidelines:

https://fedoraproject.org/wiki/Packaging:Guidelines#Filesystem_Layout

I think the best course of action is to move this file to
%{_bindir}/arduino-ctags and to patch the IDE to find it in the new location.
With this name it does not conflict with the ctags RPM and it's in the correct
location.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #8 from Gianluca Sforna <giallu(a)gmail.com&gt; ---
Ok, done here:

Spec URL: https://giallu.fedorapeople.org/arduino-ctags.spec
SRPM URL:
https://giallu.fedorapeople.org/arduino-ctags-5.8-3.arduino11.fc24.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(giallu(a)gmail.com)



--- Comment #9 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
Excellent, that looks great. There is still one thing from my first review that
hasn't been fixed and I also noticed one more thing upon looking more closely.
These must both be fixed to be approved:

[!] Package contains no bundled libraries without FPC exception.

The package contains a subset of glibc in its gnu_regex folder. According to
the packaging guidelines[0], you'll need to do a few things by my
interpretation:

* Try to get the package to work with Fedora's glibc.
* If the above is not possible for some reason, you must:
  - Put Provides: bundled(glibc) = 2.10.1 into your spec file.
  - Publicly contact upstream to request that they provide a way to use system
    glibc.
  - Document the public outreach in your spec file.


[!]: Development files must be in a -devel package

This is the one I mentioned upon my first review. You need to add an
arduino-ctags-devel package that has all the .h files, and installs them into
%{_includedir}/arduino-ctags/. However, you should not include the glibc
headers from the gnu_regex folder.


You don't have to fix this in order to pass review, but I also recommend it:

[!]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.

I recommend adding comments over your Patch0 and Patch1 lines that give a brief
description of the patch, especially the CVE patch. It's a little surprising
that the 5.8-11 release from November would not have a CVE from 2014 fixed.


[0]
https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplicat...

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Gianluca Sforna <giallu(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(giallu(a)gmail.com) |



--- Comment #10 from Gianluca Sforna <giallu(a)gmail.com&gt; ---
(In reply to Randy Barlow from comment #9)
>  The package contains a subset of glibc in its gnu_regex folder.
Apprently, that folder is used only on non POSIX systems (e.g. Windows) to
provide the functions but otherwise are not used. For good measure, I am now
removing the directory in the %prep section.


>  
>  [!]: Development files must be in a -devel package
>  
>  This is the one I mentioned upon my first review. You need to add an
>  arduino-ctags-devel package that has all the .h files, and installs them
>  into %{_includedir}/arduino-ctags/. However, you should not include the
>  glibc headers from the gnu_regex folder. 

I thought this was settled in my previous comment. I am not shipping any
library here, just a single executable so no library to link against => no
-devel needed.


>  I recommend adding comments over your Patch0 and Patch1 lines that
> give a
>  brief description of the patch, especially the CVE patch. It's a little
>  surprising that the 5.8-11 release from November would not have a CVE from
>  2014 fixed. 
Added a couple comments. Both patches are coming from the ctags Fedora package,
but I added the CVE one just to avoid bug reports later. As a matter of fact,
it affected JavaScript parsing, so it was not really relevant for this package.

Spec URL: https://giallu.fedorapeople.org/arduino-ctags.spec
SRPM URL:
https://giallu.fedorapeople.org/arduino-ctags-5.8-4.arduino11.fc24.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Gianluca Sforna <giallu(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(randy@electronswe
                   |                            |atshop.com)



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(randy@electronswe |
                   |atshop.com)                 |



--- Comment #11 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
Created attachment 1263349
  --> https://bugzilla.redhat.com/attachment.cgi?id=1263349&action=edit
review.txt

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Randy Barlow <randy(a)electronsweatshop.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+



--- Comment #12 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
Thanks for fixing the gnu_regex bit. I agree that the header files aren't
usually  used for executables - I was just thrown off by the wording from
fedora-review. I've read over the packaging guidelines and I do not believe the
headers are required by them. Thus, the package is approved!

One thing I noticed that I recommend fixing: There's a /usr/lib/.build-id
folder being created by the RPM. I recommend removing it after the make install
step. Apparently a few other RPMs do this too, so fedora-review sees it as a
conflicting owned directory.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #13 from Randy Barlow <randy(a)electronsweatshop.com&gt; ---
Nevermind my comment about the .build-id folder. I learned on the php-devel
list that it's apparently being placed there by the rpm build system and not by
your spec file:

https://lists.fedoraproject.org/archives/list/[e-mail]...

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #14 from Gianluca Sforna <giallu(a)gmail.com&gt; ---
Thank you very much Randy. If you ever need a review for one of your packages,
feel free to ping me.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #15 from Jon Ciesla <limburgher(a)gmail.com&gt; ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/arduino-ctags

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #16 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc26 has been submitted as an update to Fedora
26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-2ca713db83

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Fedora Update System <updates(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #17 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc25 has been submitted as an update to Fedora
25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-492bf566f8

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Fedora Update System <updates(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA



--- Comment #18 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc25 has been pushed to the Fedora 25 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-492bf566f8

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #19 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc26 has been pushed to the Fedora 26 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-2ca713db83

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555

Fedora Update System <updates(a)fedoraproject.org&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2017-03-27 20:25:01



--- Comment #20 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc25 has been pushed to the Fedora 25 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component

[Bug 1422555] Review Request: arduino-ctags - A mix of ctags and anjuta-tags for the perfect C++ ctags

https://bugzilla.redhat.com/show_bug.cgi?id=1422555



--- Comment #21 from Fedora Update System <updates(a)fedoraproject.org&gt; ---
arduino-ctags-5.8-4.arduino11.fc26 has been pushed to the Fedora 26 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component