Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt to spotify-make, various fixes #14

Closed
wants to merge 9 commits into from
Closed

Adapt to spotify-make, various fixes #14

wants to merge 9 commits into from

Conversation

leamas
Copy link

@leamas leamas commented Jan 14, 2013

As previous request, rebased to current spotify-make. Notes:

  • The use of spotify-make gives a cleaner spec IMHO
  • The filtering is partly broken, see separate commit
  • The use of the register.sh scripts doesn't really work when packaging - current installs
    stuff in scriptlets which becomes unowned by the package.
  • The suse-update-desktop macro is resurrected from last pull request.

@aspiers
Copy link
Owner

aspiers commented Jan 15, 2013

Thanks a LOT for this great work! It looks good at a first glance - I'm pretty busy at the moment but hopefully will find some time to test and merge soon. Thanks again :)

@leamas
Copy link
Author

leamas commented Jan 15, 2013

You're welcome :) I just ran it on upcoming 12.3 (factory) with no problems, besides some overall things to get the system up & running.

@leamas
Copy link
Author

leamas commented May 7, 2013

Now with 0.9.0 the Fedora spec and this pull request share all download/build/install code using spotify-make.

Could you possibly find the time to review and eventually merge this? As of now, I have published my own repo as this is the only one supporting 0.9.0. This is not that satisfactory, it's just meant to be a branch, not something public visible..

Wrote similar things in the wrong place, sorry for the mess.

@aspiers
Copy link
Owner

aspiers commented May 19, 2013

Thanks a lot for this. I've finally had a look and in general it looks really good. I really really like your cross-platform spotify-make which cleanly abstracts the installation from the packaging :-) This is a good way to go since it gives a uniform experience for packagers across distributions.

There is currently one quite big problem however - it no longer works ;-) I'll comment on the individual commits to explain why ...

@@ -0,0 +1,5 @@
This package was built by the openSUSE Spotify installer; see
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with introducing a new README file is that someone who clones the git repo will get confused between README and README.md. So this should be called something like spotify-client-package-README.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

@aspiers
Copy link
Owner

aspiers commented May 19, 2013

Final comments (for now): the pull request doesn't update README.md at all. It should:

  • update references to 12.2 to clarify which of 12.2, 12.3, and Factory are supported
  • explain there is a new dependency on spotify-make, and why that is a good idea (I am already convinced but other people need to be convinced too)
  • ensure that the listed installation procedures work out of the box.

This last point is the most important by far. The main goal of this installer is to make installation a no brainer for non-technical people and remove the need for wading through those awful web forums. Your changes are generally good and I would love to merge them, but I can't until they stop breaking the existing "no brain required" installation process. Thanks a lot again for your work on this!

@leamas
Copy link
Author

leamas commented May 23, 2013

Been away, as you might have noticed. Will look into this.

I left the spec file and the README as-is in the intention you would like to fix this yourself. This was obviously a mistake which I will correct.

Totally agree that the installation procedure should work out of the box (Why is there not a prebuilt repo package to download?).

Alec Leamas and others added 9 commits May 23, 2013 14:25
Use installer from https://github.com/leamas/spotify-make to
install package. This package cleanly abstracts the installation
from the packaging and  gives a uniform experience for packagers
across distributions. With this commit, it's used both by SUSE
and Fedora.

New installation paths in /usr/share and /usr/lib[64].
Scriptlets chamged to old standard macros, using the register
shellscript doesn't work in a packaging context where we need to
install files in %install and run gtk-update-icon-cache in the
scriptlets (old code actually installed files in the scriptlets!).

ldconfig removed, no spotify library must ever enter the ld.so cache...

Cleaning up dependencies
0.8.8 has a bad rpath in the binary, making it necessary to disable
the default rpath checking. Couldn't make the original NO_BRP_CHECK_RPATH
work, doing it the hard way insted.
With the rest of the spec cleaned up, these clutters too much.
We need to handle the deps for libcrypto/libssl. Install script
will resolve the 0.9.8 requires as 1.0.0 iff 1.0.0 is present
and 0.9.8 is *not* present during build. This is too complicated
to fix, so we just add some hard links after 'make install' to
always use 1.0.0 according to mysterious entry in changelog.

Current filtering is broken. It Provides: libcef.so, making it possible
for other apps to link to that library. It also turns off the
dependency generator which makes zypper happily install package
with all sorts of dependency errors unless we state all of them
explicitly (how has this ever worked?).

Removing complete filter solution, using upstream builtin filtering
instead. Requires us to add som manual Requires: for symlinked deps
which not are picked up automatically. No solution is really optimal
here, but this is the best I have found out so far.
The mandriva are not maintained and probably little help should
someone step up to make a mandriva spec. They also adds clutter
somewhat. For the fedora ones, there is already a spec in much
better shape.
Although we havn't the sources, it makes sense to package a complete
srpm which can be used on any supported architecture.
The 0.8.8 release contained a bad rpath, which required code to
handle (i. e., basically accept) it. Since the 0.9.0 does not
have this bad rpath the checking can be restored to the sane
defaults.
Major update. Don't require spec file to be installed, download it instead.
Use set -e instead of home-brewed safe-run. Use rpmdev-setuptree to create
a personal build environment if needed (i. e., so user can build without
root privileges). Only require root privileges to install/uninstall. Use
spotify-make to download .deb packages. Use rpmdev-spectool to download
other sources. Don't hardcode locations to %_srcdir, %_topdir etc.

As a consequence of above, the spec is excluded from the spec file and
the README updated.
@leamas
Copy link
Author

leamas commented May 24, 2013

Closing, new pull request under way

@leamas leamas closed this May 24, 2013
@leamas leamas reopened this May 24, 2013
@leamas leamas closed this May 24, 2013
@aspiers
Copy link
Owner

aspiers commented May 24, 2013

Thanks for working on this! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants