Bug 848211 (mirall)

lets have a look:

- owncloud-client.desktop: error: value "Utility" for string list key "Categories" in group "Desktop Entry" does not have a semicolon (';') as trailing character

- BuildRequires:  oxygen-icon-theme
Really?

-Requires:       net-tools
-Requires:       iproute
I've never used mirall before, but this can't be right. iproute is a replacement for net-tools. Why would you install both?

-again the changelog is a little bit borked: please insert the appropriate versions

-%config %{_bindir}/../../etc/exclude.lst
just use configdir ;) Actually this file looks quite lost. Can you also put it in /etc/mirall or rename it? and you want to use noreplace, too

-there are .so files in libdir: please call ldconfig in post and postun

-drop defattr

-BuildRequires:  qt4-devel >= 4.7
You can remove that check. even f16 ships qt 4.8

- I think you can also remove the first two lines in the description. This "header" is not really necessary there

And it would be great if could you upgrade to the new mirall release

Thanks :)

Gregor thank you for the help,
this is the new versions fixing all the above and updating to mirall-1.0.5

Spec URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall.spec
SRPM URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall-1.0.5-1.fc17.src.rpm

the previous files are now moved to:
http://jmarrero.fedorapeople.org/packages/mirall/old/*

Let me know if I missed something.

a more in deep look:

- the package contains desktop file icons - you must update the icon cache:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

- mirall.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmirallsync.so /lib64/libQtTest.so.4...
-> again set -Wl,--as-needed

- upstream names the owncloud-client binary just owncloud. This is dumb and much too generic. Maybe owncloud-sync-client would be more appropriate...be creative but don't forget the desktop file

- actually this package ships two apps: a sync client and some kind of installation tool. Would it make sense to split it up? I how far are those 2 apps related? Can you use one without the other?

- blocker:
mirall.x86_64: E: invalid-soname /usr/lib64/libmirallsync.so libmirallsync.so
The soname of the library is neither of the form lib<libname>.so.<major> or
lib<libname>-<major>.so.
mirall.x86_64: E: invalid-soname /usr/lib64/libowncloudsync.so libowncloudsync.so
The soname of the library is neither of the form lib<libname>.so.<major> or
lib<libname>-<major>.so.

->I suppose these are not dev libs, so they must be versioned

- "- moved exclude.lst to /etc/mirall/"
Is it that easy? Are you sure the config file is still read by the client?
Oh, and mark it as %config

- BuildRequires:  inetd desktop-file-utils
Builds fine without inetd?

keep up :)

> - blocker:
> mirall.x86_64: E: invalid-soname /usr/lib64/libmirallsync.so libmirallsync.so
> The soname of the library is neither of the form lib<libname>.so.<major> or
> lib<libname>-<major>.so.
> mirall.x86_64: E: invalid-soname /usr/lib64/libowncloudsync.so
> libowncloudsync.so
> The soname of the library is neither of the form lib<libname>.so.<major> or
> lib<libname>-<major>.so.
>
> ->I suppose these are not dev libs, so they must be versioned

Says what guideline?

Lack of library versioning is an upstream issue. I am not aware of any Fedora guideline requiring shared libraries to be versioned, and in fact I've been told multiple times that Fedora does NOT recommend inventing our own library versioning because the versioned sonames we come up with might conflict with later upstream ones.

It IS recommended that shared libraries be versioned, but it must be done by upstream.

this is the new version
I splited the apps, to mirall-common wich is a dependency of owncloud-client and mirall.

Tested owncloud and mirall and they work just fine without each other installed. 
mirall common holds config file (/etc/exclude.lst) and lang.

Spec URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall.spec
SRPM URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall-1.0.5-2.fc17.src.rpm

the previous files are moved to:
http://jmarrero.fedorapeople.org/packages/mirall/old/*

asked uptream about the posiblity of moving /etc/exclude.lst to /etc/mirall/exclude.lst and adding versioning to the libs.

Very well, the objective is now to make use of the new owncloud-csync libs. For that you have to patch the buildsys again. I've attached a small patch...something in this way should do the job

And can you comment please on that BuildReq to xinetd?

(In reply to comment #5)
> asked uptream about the posiblity of moving /etc/exclude.lst to
> /etc/mirall/exclude.lst and adding versioning to the libs.
excellent

Created attachment 605354 [details]
lib rename patch

I'll do the formal review today, since no one's assigned the bug or set the review flag yet.

naming: ok

sources: ok
$ md5sum mirall*.bz2
b7a96411f092bb16f88e3868a558032f  mirall-1.0.5.tar.bz2

1. MUST: build fails, currently does not build against the renamed owncloud-csync, will need some variant of  the patch from comment #7

2. MUST: missing icon scriptlets for owncloud-client subpkg

oh,  and please document/justify (or remove) the following dependencies:
BuildRequires: inet
Requires: iproute
Requires: oxygen-icon-theme

Spec URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall.spec
SRPM URL: http://jmarrero.fedorapeople.org/packages/mirall/mirall-1.0.5-3.fc17.src.rpm

http://jmarrero.fedorapeople.org/packages/mirall/old/*

licensing: ok

scriptlets:  ok

macros: ok

3. SHOULD remove 
-DCMAKE_INSTALL_PREFIX=%{_prefix} -DCMAKE_INSTALL_SYSCONFDIR=%{_sysconfdir}
the default %cmake macro already includes these

otherwise, good work.  3 can be fixed post-review.


APPROVED.

New Package SCM Request
=======================
Package Name: mirall
Short Description: owncloud desktop client
Owners: jmarrero
Branches: f17 f18

Git done (by process-git-requests).

owncloud-csync-0.50.8-8.fc17,mirall-1.0.5-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/owncloud-csync-0.50.8-8.fc17,mirall-1.0.5-3.fc17

owncloud-csync-0.50.8-8.fc18,mirall-1.0.5-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/owncloud-csync-0.50.8-8.fc18,mirall-1.0.5-3.fc18

owncloud-csync-0.50.8-8.fc17, mirall-1.0.5-3.fc17 has been pushed to the Fedora 17 testing repository.

owncloud-csync-0.50.8-9.fc17,mirall-1.0.5-4.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/owncloud-csync-0.50.8-9.fc17,mirall-1.0.5-4.fc17

owncloud-csync-0.50.8-9.fc18,mirall-1.0.5-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/owncloud-csync-0.50.8-9.fc18,mirall-1.0.5-4.fc18

owncloud-csync-0.50.8-9.fc17, mirall-1.0.5-4.fc17 has been pushed to the Fedora 17 stable repository.

owncloud-csync-0.50.8-9.fc18, mirall-1.0.5-4.fc18 has been pushed to the Fedora 18 stable repository.