Bug 1069048 - minor spec file improvements: for README and symlink
Summary: minor spec file improvements: for README and symlink
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: ShellCheck
Version: 20
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Dridi Boukelmoune
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-24 03:14 UTC by Jens Petersen
Modified: 2014-04-14 22:47 UTC (History)
2 users (show)

Fixed In Version: ShellCheck-0.3.1-6.fc20
Clone Of:
Environment:
Last Closed: 2014-04-14 22:45:48 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
ShellCheck.spec-1.patch (934 bytes, patch)
2014-02-24 03:14 UTC, Jens Petersen
no flags Details | Diff
switch to dynamic linking (758 bytes, patch)
2014-02-27 07:44 UTC, Dridi Boukelmoune
no flags Details | Diff

Description Jens Petersen 2014-02-24 03:14:07 UTC
Created attachment 866852 [details]
ShellCheck.spec-1.patch

Description of problem:
The attached patch makes some small tweaks to make the spec safer:

- only create symlink if one doesn't exist already
- separately package LICENSE/README for ShellCheck and ghc-ShellCheck
  to avoid update issues

The latter prevents update issues if you try to update
ShellCheck and ghc-ShellCheck* separately for some reason.

Comment 1 Jens Petersen 2014-02-24 03:21:51 UTC
For reference, this is the error message with *ShellCheck* already installed:

$ sudo yum update ghc-ShellCheck*

:

Transaction check error:
  file /usr/share/doc/ShellCheck/README from install of ghc-ShellCheck-0.3.1-3.fc20.x86_64 conflicts with file from package ShellCheck-0.2.0-3.fc20.x86_64
  file /usr/share/doc/ShellCheck/README from install of ghc-ShellCheck-devel-0.3.1-3.fc20.x86_64 conflicts with file from package ShellCheck-0.2.0-3.fc20.x86_64

Comment 2 Dridi Boukelmoune 2014-02-24 07:51:13 UTC
Hi,

Thanks for the patch and I hope you enjoy the packages :)

Would it make more sense to have ShellCheck strictly depend on ghc-ShellCheck ?

It seems that the main package doesn't link to its shared library but embeds it statically instead. That shouldn't happen according to the main guidelines, but it doesn't seem to be a problem according to the Haskell guidelines (no Requires tag in the `Library and Binary' template).

Comment 3 Jens Petersen 2014-02-25 02:37:10 UTC
> Would it make more sense to have ShellCheck strictly depend on
> ghc-ShellCheck ?

Yes, but it should be done at the .cabal level
not explicitly in the spec file.

Pandoc does this for example and so does Agda:
it is particularly desirable large packages.

> It seems that the main package doesn't link to its shared library but embeds
> it statically instead. That shouldn't happen according to the main
> guidelines, but it doesn't seem to be a problem according to the Haskell
> guidelines (no Requires tag in the `Library and Binary' template).

Right - it needs some massaging of the .cabal file, etc.
It should not be very hard but one might need to experiment a bit.
In theory adding "depends: ShellCheck to the executable section
ought to be enough but in practice it doesn't seem to be.
Either way I would be curious to see the final changes that make it work
since I would also like to know how to do it, but if you
run into difficulties I can also try to help with it.
Looking at the other examples hopefully we can work it out.

Comment 4 Jens Petersen 2014-02-25 02:40:33 UTC
> That shouldn't happen according to the main guidelines,
> but it doesn't seem to be a problem according to the Haskell
> guidelines (no Requires tag in the `Library and Binary' template).

(Right it is not mandatory for Fedora Haskell packages
and by default Cabal does not self-link executables.)

Comment 5 Dridi Boukelmoune 2014-02-27 07:44:44 UTC
Created attachment 868345 [details]
switch to dynamic linking

I've attached a quick n dirty patch for my spec that switches to dynamic linking.

That should solve the upgrade issue:
$ rpm -qpR /var/lib/mock/fedora-rawhide-x86_64/result/ShellCheck-*.x86_64.rpm | grep ShellCheck
ghc(ShellCheck-0.3.1-d837c7bb20080c4086df07bc61506f2a)
libHSShellCheck-0.3.1-ghc7.6.3.so()(64bit)

If it sounds good to you I'll try to upstream my modifications to the cabal file and update the package in rawhide.

Comment 6 Jens Petersen 2014-02-27 07:59:56 UTC
Yes, the changes to the .cabal look good.
I think it should be good for upstream.

(If you want to build that first for Rawhide
you should do it as a patch really.)

Comment 7 Jens Petersen 2014-02-27 08:01:30 UTC
So after that anyway it should be sufficient just to have the 2 doc files in ghc-ShellCheck. :)

Comment 8 Dridi Boukelmoune 2014-02-27 13:10:48 UTC
(In reply to Jens Petersen from comment #6)
> (If you want to build that first for Rawhide
> you should do it as a patch really.)

Of course, I posted the "quick n dirty" fix just to make sure we both agree on this solution.

I'll make a proper patch and send this uptsream, so that I can link to a URL in upstream's issue tracker once I integrate the patch for Fedora.

Comment 9 Dridi Boukelmoune 2014-03-01 17:31:24 UTC
Patch merged upstream:
https://github.com/koalaman/shellcheck/pull/105
https://github.com/koalaman/shellcheck/pull/105.diff

Update coming soon in rawhide !

(In reply to Jens Petersen from comment #7)
> So after that anyway it should be sufficient just to have the 2 doc files in
> ghc-ShellCheck. :)

I'd rather keep the files in the main package (ShellCheck) pkgdocdir.

Comment 10 Dridi Boukelmoune 2014-03-01 18:43:21 UTC
Build successful:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6583468

Comment 11 Fedora Update System 2014-03-01 19:26:56 UTC
ShellCheck-0.3.1-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/ShellCheck-0.3.1-4.fc20

Comment 12 Fedora Update System 2014-03-01 19:27:05 UTC
ShellCheck-0.3.1-4.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/ShellCheck-0.3.1-4.fc19

Comment 13 Fedora Update System 2014-03-03 03:09:31 UTC
Package ShellCheck-0.3.1-4.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ShellCheck-0.3.1-4.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-3342/ShellCheck-0.3.1-4.fc20
then log in and leave karma (feedback).

Comment 14 Jens Petersen 2014-03-10 10:31:30 UTC
My apologies - you need one more thing in ShellCheck.spec
for the selflinking of the executable to work.
Namely need to fix the rpath to the ShellCheck lib.
I have a macro for that so if you add

%ghc_fix_dynamic_rpath shellcheck

towards the end of the %install section it should avoid this
runtime error in ShellCheck-0.3.1-4.fc20:

$ shellcheck 
shellcheck: error while loading shared libraries: libHSShellCheck-0.3.1-ghc7.6.3.so: cannot open shared object file: No such file or directory

Sorry I forgot to mention that earlier: to prevent this
bad build going stable I undid the stable requests in Bodhi.
(Just noticed this by chance since I wanted to use
shellcheck on a script now, I should have tested more carefully
earlier.:)

Comment 15 Dridi Boukelmoune 2014-03-16 15:34:51 UTC
For some reason I got an email notification for your bodhi comment but not this one. I solved it with a drop-in conf for ld, I'll fall back and use your macro instead.

Comment 16 Dridi Boukelmoune 2014-03-16 16:08:14 UTC
I've tried to use the macro but it doesn't seem to work:
===
@@ -79,6 +79,7 @@ make shellcheck.1
 
 %install
 %ghc_lib_install
+%ghc_fix_dynamic_rpath shellcheck
 
 # currently no pandoc on arm (bug #992430)
 %ifnarch %{arm}
===

The rest of the install section is just a copy of the man page (except on arm currently).

And then when I check:
ldd /usr/bin/shellcheck | grep ShellCheck
	libHSShellCheck-0.3.1-ghc7.6.3.so => not found

ShellCheck-0.3.1-5 should fix this, can you have a look at it once it's available on rawhide ?
http://koji.fedoraproject.org/koji/taskinfo?taskID=6637764

Comment 17 Jens Petersen 2014-03-18 01:33:32 UTC
(In reply to Dridi Boukelmoune from comment #16)
> I've tried to use the macro but it doesn't seem to work:

> +%ghc_fix_dynamic_rpath shellcheck

> And then when I check:
> ldd /usr/bin/shellcheck | grep ShellCheck
> 	libHSShellCheck-0.3.1-ghc7.6.3.so => not found

You're right: this is again because $(cwd) != %{pkg_name}-%{version} :-|
It would really make life (and you .spec file;) a lot easier if
you could move to the hackage tarball...

> ShellCheck-0.3.1-5 should fix this, can you have a look at it once it's
> available on rawhide ?
> http://koji.fedoraproject.org/koji/taskinfo?taskID=6637764

Okay that did workaround the rpath issue
but I don't think it is nice leaving the incorrect
RPATH in shellcheck.  So I added a "modified"
%ghc_fix_dynamic_rpath to the spec file in
ShellCheck-0.3.1-6.fc21 which takes into account
the source dir name.  I suppose arguably it could be
a ghc-rpm-macros bug but this package is the only
exception currently.  It seems to work okay for me
anyway.

Comment 18 Dridi Boukelmoune 2014-03-18 08:35:17 UTC
I really need to fix bug #1052117 then, because the tarball from hackage is still not usable in its current state.

Thanks for the workaround.

Comment 19 Fedora Update System 2014-03-31 13:30:43 UTC
ShellCheck-0.3.1-6.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/ShellCheck-0.3.1-6.fc19

Comment 20 Fedora Update System 2014-03-31 13:30:52 UTC
ShellCheck-0.3.1-6.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/ShellCheck-0.3.1-6.fc20

Comment 21 Fedora Update System 2014-04-14 22:45:48 UTC
ShellCheck-0.3.1-6.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2014-04-14 22:47:42 UTC
ShellCheck-0.3.1-6.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.


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