Bug 839851 (mate-common) - Review Request: mate-common -- mate common build files
Summary: Review Request: mate-common -- mate common build files
Keywords:
Status: CLOSED ERRATA
Alias: mate-common
Product: Fedora
Classification: Fedora
Component: mate-common
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Dan Mashal
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: MATE-DE-tracker
TreeView+ depends on / blocked
 
Reported: 2012-07-13 03:25 UTC by Dan Mashal
Modified: 2012-10-23 19:28 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-07-19 09:09:35 UTC
Type: Bug
Embargoed:
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Dan Mashal 2012-07-13 03:25:42 UTC
Spec URL: http://vicodan.fedorapeople.org/mate-common.spec
SRPM URL: http://vicodan.fedorapeople.org/mate-common-1.4.0-1.fc17.src.rpm
Description: BitchX: MATE dekstop engine

Comment 1 Dan Mashal 2012-07-13 03:27:40 UTC
oops minus the BitchX sorry, about that.. copy pasting.

Comment 2 Rex Dieter 2012-07-13 03:30:09 UTC
I believe you're already sponsored... :) removing FE-NEEDSPONSOR, and adjusting summary to match pkg %name and %summary

Comment 3 Dan Mashal 2012-07-13 03:30:57 UTC
Thank you Rex. Care to do the review? ;)

Comment 4 Rex Dieter 2012-07-13 03:32:02 UTC
initial commments:

no need to BuildRequires: gcc

otherwise, the BuildRequires: are quite sparse, really, nothing?

and, using a /usr/local prefix here is (probably) not acceptable, and try to use the default /usr prefix and %configure macro

Comment 5 Dan Mashal 2012-07-13 03:41:07 UTC
mate-common is the first basic component of mate so all that's required is python auto make and gcc really. I can do fresh reinstall of fedora to double check but I just did that. As for configure mate uses autogen.sh and does not come with a configure script. Usr directory noted.

Comment 6 Dan Mashal 2012-07-13 13:04:40 UTC
All 21 RPMs are built. Will upload to fedora people and polish spec files for you within 24 hours sir. Thanks for your help.

Comment 7 Ankur Sinha (FranciscoD) 2012-07-13 13:19:41 UTC
(In reply to comment #5)
> mate-common is the first basic component of mate so all that's required is
> python auto make and gcc really. I can do fresh reinstall of fedora to
> double check but I just did that. As for configure mate uses autogen.sh and
> does not come with a configure script. Usr directory noted.

Dan,

You can just use mock or fire scratch builds at koji now that you're a packager. They use clean build environments and help in catching build requires. 


http://fedoraproject.org/wiki/Using_Mock_to_test_package_builds

Since MATE is a big collection of packages and would require quite a few reviews, is it worth making a tracker bug for it that would help track the reviews for its components?

I'll help with the reviewing when I find free cycles.

Thanks,
Ankur

Comment 8 Rex Dieter 2012-07-13 13:36:35 UTC
so, here's some suggested fixes:
http://rdieter.fedorapeople.org/rpms/mate/

%changelog
* Fri Jul 13 2012 Rex Dieter <rdieter> 1.4.0-2
- omit Group: tag
- fix URL, Source0
- use %%configure macro
- BuildArch: noarch

the stuff about urls, and %configure macro usage you ought to adopt in your other mate-related pkgs too.

and scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4238369

Comment 9 Nelson Marques 2012-07-13 14:25:13 UTC
Anyone against that I help with this review ?

Comment 10 Dan Mashal 2012-07-13 14:34:35 UTC
Hi,

All RPMs, SRPMs and SPEC files have been built and are uploaded here:

http://vicodan.fedorapeople.org/

Please review.

This will need legal review as well.

It was brought to my attention "nyan cat" is copyrighted.

Thanks,
Dan

Comment 11 Mario Blättermann 2012-07-13 21:42:02 UTC
(In reply to comment #7)
> Since MATE is a big collection of packages and would require quite a few
> reviews, is it worth making a tracker bug for it that would help track the
> reviews for its components?

Done. Tracker is bug #840149.

Comment 12 Dan Mashal 2012-07-13 22:00:13 UTC
Thank you Rex and Mario.

Rex, I will work with your suggestions this weekend.

Mario, thanks for creating the tracker.

Comment 13 Nelson Marques 2012-07-14 15:25:03 UTC
Hi all,

I've looked around and it seems that nyan cat is indeed copyrighted and there's currently 3 copyright holders:

 1) 1 copyright holder for the animated gif;
 2) 1 copyright holder for the music;
 3) 1 copyright holder for the video;

I've taken the liberty of opening a bug report upstream requesting for this issue to be cleared out; I will talk to Stefano and Perberos about this once I grab them online.

Upstream: https://github.com/perberos/Mate-Desktop-Environment/issues/104

Comment 14 Nelson Marques 2012-07-14 20:18:14 UTC
Aditional Information:

You can disable 'nyan cat' during build time if you want, you need to add this option to %configure (which you are neglecting currently):

%build
%configure --disable-nyancat

And that should get rid of nyancat.

Comment 15 Nelson Marques 2012-07-14 20:27:32 UTC
Dan,

I've found that also another person has a few very neat packages for Fedora; You can use them to improve your specs if you want or even contact the original author, tell him about your effort and maybe you guys can do this together.

Take a look at this: https://github.com/mate-desktop/fedora-packages

Comment 16 Dan Mashal 2012-07-15 00:32:28 UTC
Nelson,

Thanks for the Nyan cat comment.

Regarding those packages, they are out of date and heavily patched.

I have been working with Nice&Gently. He is on vacation right now.

Thanks,
Dan

Comment 17 Tom "spot" Callaway 2012-07-15 18:53:30 UTC
We probably cannot distribute "nyan cat" in source format either, without permission from the copyright holder. I have not looked at the source code at all, however, if only the image of "pop tart cat" is being used here, you may wish you ask the copyright holder (prguitarman <pr>) for permission.

The alternative would be to simply remove the "nyan cat" content from the source tarball entirely and make a "clean" tarball.

Comment 20 Dan Mashal 2012-07-16 06:45:24 UTC
one more try for srpm link:

http://vicodan.fedorapeople.org/materpms/srpms/mate-common-1.4.0-3.src.rpm

Sorry.

Comment 21 Dan Mashal 2012-07-16 22:39:07 UTC
Hi Rex,

Per our conversation on IRC I have updated the SPEC and SRPM. Please review it. 

Once mate-common gets approved I can make mate-common a requirement for every other spec. We will also track any legality issues in the main tracker bug.

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-common.spec
SRPM: http://vicodan.fedorapeople.org/materpms/srpms/mate-common-1.4.0-4.fc17.src.rpm
Description: base files for building and installing MATE Desktop

Comment 22 Rex Dieter 2012-07-16 23:04:45 UTC
So, I just looked closer at the gnome-common analog,
http://pkgs.fedoraproject.org/gitweb/?p=gnome-common.git;a=blob;f=gnome-common.spec

and see some things we could... borrow here. :)

stuff like:
runtime requires
better summary/description

so,

1.  MUST: add runtime deps
Requires: automake autoconf libtool gettext pkgconfig

2.  SHOULD: improve pkg summary/description as suggested

3.  MUST: Licensing:  so, .spec says GPLv2+, and none of the included files mention licensing, except for the embedded COPYING file, which is GPLv3.  can you verify with upstream (if you haven't already), their intent here?  (I'm assuming some GPLv2+ (with some/all libs LGPLv2+) combo like pre-forked gnome2...)

naming: ok
macros: ok
scriptlets: n/a

sources: ok
$ md5sum *.xz
bc49ff6897ef2303c6464a3ca46aaf35  mate-common-1.4.0.tar.xz

Comment 23 Dan Mashal 2012-07-16 23:48:52 UTC
Updated license. It is GPLv3+. 

I will do the license review on a package by package basis. Most everything is GPLv2+, GPLv3(+), lGPL as per perberos.

Added gnome-common to requires field.

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-common.spec
SRPM: http://vicodan.fedorapeople.org/materpms/srpms/mate-common-1.4.0-5.fc17.src.rpm

Comment 24 Rex Dieter 2012-07-17 01:51:53 UTC
OK, looks good, APPROVED.

curious why you added
Requires: gnome-common
though. ??

Comment 25 Dan Mashal 2012-07-17 01:54:10 UTC
My mistake, misunderstood. I'll remove that. Thanks!

Comment 26 Dan Mashal 2012-07-17 02:15:32 UTC
New Package SCM Request
=======================
Package Name: mate-common
Short Description: binaries for building all MATE desktop sub components
Owners: vicodan rdieter
Branches: f16 f17
InitialCC: vicodan rdieter

Comment 27 Gwyn Ciesla 2012-07-18 03:24:11 UTC
Git done (by process-git-requests).

Comment 28 Fedora Update System 2012-07-18 05:22:28 UTC
mate-common-1.4.0-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-4.fc16

Comment 29 Fedora Update System 2012-07-18 05:22:37 UTC
mate-common-1.4.0-4.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-4.fc17

Comment 30 Dan Mashal 2012-07-18 05:23:57 UTC
Thanks Jon.

Comment 31 Fedora Update System 2012-07-18 06:45:28 UTC
mate-common-1.4.0-5.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-5.fc17

Comment 32 Fedora Update System 2012-07-18 06:45:39 UTC
mate-common-1.4.0-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-5.fc16

Comment 33 Fedora Update System 2012-07-19 09:09:35 UTC
mate-common-1.4.0-5.fc17 has been pushed to the Fedora 17 stable repository.

Comment 34 Fedora Update System 2012-07-19 09:14:26 UTC
mate-common-1.4.0-5.fc16 has been pushed to the Fedora 16 stable repository.

Comment 35 Wolfgang Ulbrich 2012-07-19 13:44:13 UTC
A little correction from me.

BuildRequires: automake
BuildRequires: autoconf

Requires: automake
Requires: autoconf
Requires: libtool
Requires: gettext
Requires: pkgconfig

because for building you need only automake and autoconf
but for a working package after installation we need all the requires.

Comment 36 Rex Dieter 2012-07-19 13:48:33 UTC
Oops indeed, missed my MUST item 1 above about adding the Requires:

Comment 37 Wolfgang Ulbrich 2012-07-24 22:29:07 UTC
Can you update the requires like in my comment 35 and release an update?
Otherwise you have to add the missing Requires to every other package which need them.
The sense of mate-common is this not to do for every other package.

Comment 38 Dan Mashal 2012-07-24 22:33:21 UTC
Sure.

Comment 39 Wolfgang Ulbrich 2012-07-24 22:38:16 UTC
I give you  a example:
building libmatekeyring with fedora's mate-common

DEBUG: + cd libmatekeyring-1.4.0
DEBUG: + /bin/chmod -Rf a+rX,u+w,g-w,o-w .
DEBUG: + NOCONFIGURE=1
DEBUG: + ./autogen.sh
DEBUG: /usr/bin/mate-autogen
DEBUG: checking for autoconf >= 2.53...
DEBUG:   testing autoconf2.50... not found.
DEBUG:   testing autoconf... found 2.68
DEBUG: checking for automake >= 1.9...
DEBUG:   testing automake-1.12... not found.
DEBUG:   testing automake-1.11... found 1.11.1
DEBUG: checking for libtool >= 1.4.3...
DEBUG:   testing libtoolize... not found.
DEBUG: ***Error***: You must have libtool >= 1.4.3 installed

This is why Requires: libtool is missing in mate-common

Pls update

Comment 40 Fedora Update System 2012-07-25 01:15:27 UTC
mate-common-1.4.0-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-6.fc17

Comment 41 Fedora Update System 2012-07-25 01:15:33 UTC
mate-common-1.4.0-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-6.fc16

Comment 42 Dan Mashal 2012-07-25 01:16:11 UTC
Done.

Please test and leave karma.

Comment 43 Wolfgang Ulbrich 2012-07-25 01:40:17 UTC
yeap, it works.
no need to to add BuildRequires: libtool for libmatekeyring
I will leave positiv karma.

thx Dan

Comment 44 Fedora Update System 2012-08-02 21:38:53 UTC
mate-common-1.4.0-8.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-8.fc17

Comment 45 Fedora Update System 2012-08-02 21:39:04 UTC
mate-common-1.4.0-8.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-common-1.4.0-8.fc16

Comment 46 Fedora Update System 2012-08-03 11:28:03 UTC
mate-common-1.4.0-8.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 47 Fedora Update System 2012-08-03 11:30:09 UTC
mate-common-1.4.0-8.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 48 Wolfgang Ulbrich 2012-08-03 11:48:27 UTC
Dan, pls give me a favor.
Change Requires like in version 1.4.0-6
We need pkgconfig here, and we don't need intltool glib2-devel gtk-doc for every other mate application.
Do it like this.

BuildRequires:	automake autoconf
Requires: automake autoconf libtool gettext pkgconfig

Comment 49 Matthias Runge 2012-08-03 11:58:40 UTC
uhm, 
you're requiring automake autoconf libtool and pkgconfig at runtime, and not during build? Really? 

Taken from here:
http://vicodan.fedorapeople.org/matespec/mate-common.spec

Comment 50 Wolfgang Ulbrich 2012-08-03 12:37:52 UTC
(In reply to comment #49)
> uhm, 
> you're requiring automake autoconf libtool and pkgconfig at runtime, and not
> during build? Really? 
> 
> Taken from here:
> http://vicodan.fedorapeople.org/matespec/mate-common.spec

This is the sense of mate-common, adding some main requries for building mate packages.
Otherwise we have to add automake autoconf libtool gettext and pkgconfig as BuildRquires in every other package.

Comment 51 Wolfgang Ulbrich 2012-08-03 12:42:47 UTC
(In reply to comment #50)
> (In reply to comment #49)
> > uhm, 
> > you're requiring automake autoconf libtool and pkgconfig at runtime, and not
> > during build? Really? 
> > 
> > Taken from here:
> > http://vicodan.fedorapeople.org/matespec/mate-common.spec
> 
> This is the sense of mate-common, adding some main requries for building
> mate packages.
> Otherwise we have to add automake autoconf libtool gettext and pkgconfig as
> BuildRquires in every other package.

For running ./autogen.sh for building configure and make files of mate packages, you need this requires.

Comment 52 Matthias Runge 2012-08-03 13:04:47 UTC
Thank you for your explanation.

I absolutely understand, why requirements are necessary. 

The difference between BuildRequirements and Requirements is, that you need BuildRequire during build, requirements are not needed for build, but during runtime. 

Sadly, this didn't answer, why automake autoconf etc. are not required during build time, but for runtime.

Comment 53 Wolfgang Ulbrich 2012-08-03 13:25:12 UTC
(In reply to comment #52)
> Thank you for your explanation.
> 
> I absolutely understand, why requirements are necessary. 
> 
> The difference between BuildRequirements and Requirements is, that you need
> BuildRequire during build, requirements are not needed for build, but during
> runtime. 
> 
> Sadly, this didn't answer, why automake autoconf etc. are not required
> during build time, but for runtime.

???
If i follow your link was given in comment 49,
i see
BuildRequires:	automake autoconf

Comment 54 Matthias Runge 2012-08-03 17:17:57 UTC
> ???
> If i follow your link was given in comment 49,
> i see
> BuildRequires:	automake autoconf

from the link:

BuildRequires:	automake autoconf
Requires: automake autoconf libtool gettext pkgconfig

So: why automake autoconf libtool gettext pkgconfig are required at runtime opposed to build time?

Comment 55 Dan Mashal 2012-08-03 20:27:03 UTC
These are required at run time because mate-common provides mate-autogen which requires those at run time.

Comment 56 Matthias Runge 2012-08-06 07:24:55 UTC
OK, I was mislead by the name of the package. 
It is required only for compiling other mate packages and not required at mate run-time, right? 

If yes, it probably should be named mate-devel, see
https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages:

Fedora packages must be designed with a logical separation of files. Specifically, -devel packages must be used to contain files which are intended  solely for development or needed only at build-time.

Comment 57 Dan Mashal 2012-08-06 07:26:19 UTC
That is correct. I will have to take this up with upstream, however in its current state it makes building the dependant packages a lot simpler.

Comment 58 Rex Dieter 2012-08-06 16:29:30 UTC
fyi, It's perfectly ok for some build-time/devel type packages to not be named -devel.  There are many such examples, mate-common and gnome-common being 2 of them (others include: automake, autoconf, libtool, gcc, cmake)

Comment 59 Matthias Runge 2012-08-06 18:37:51 UTC
(In reply to comment #58)
> fyi, It's perfectly ok for some build-time/devel type packages to not be
> named -devel.  There are many such examples, mate-common and gnome-common
> being 2 of them (others include: automake, autoconf, libtool, gcc, cmake)
OK; I didn't knew that. Thank you for clarification.


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