Spec URL: http://fedorapeople.org/~ynemoy/xmonad/xmonad.spec SRPM URL: http://fedorapeople.org/~ynemoy/xmonad/xmonad-0.5-1.fc8.src.rpm Description: xmonad is a tiling window manager for X. Windows are arranged automatically to tile the screen without gaps or overlap, maximising screen use. All features of the window manager are accessible from the keyboard: a mouse is strictly optional. xmonad is written and extensible in Haskell. Custom layout algorithms, and other extensions, may be written by the user in config files. Layouts are applied dynamically, and different layouts may be used on each workspace. Xinerama is fully supported, allowing windows to be tiled on several screens.
Hi, if you do not need to be sponsored, I would like to review this package.
Note this package requires ghc-X11 which is not yet in Fedora, but see bug 351361. BTW I have some (newer) packages which I made independently (probably need some cleanup for Fedora): http://haskell.org/fedora/haskell/SRPMS/xmonad-0.6-1.src.rpm http://haskell.org/fedora/haskell/SRPMS/ghc-X11-1.4.1-1.src.rpm
SPEC: http://ynemoy.fedorapeople.org/repo/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/repo/xmonad-0.7-1.fc9.src.rpm This update follows the Draft Guidelines, which are under review. Reviewing this package will help find problems in the guidelines.
Created attachment 314788 [details] xmonad.spec.patch-1 cleanup and simplification
I am tempted to drop hsc_name too, it just seems to make everything more verbose. I suggest just changing %{hsc_name} to ghc.
I've redone the guidelines completely to use macros. I'll be going over this in the coming weeks to make some badly needed updates to everything.
The templates should be made self-contained by including the necessary macros.
Theoretically yeah, but these macros are going to go in the GHC package, based on advice from the Packaging Committee.
The guidelines were approved (and hopefully will be accepted by FESCo today) but of course we need to see a ghc package rev with those macros included before reviews can really move forward. I guess this package (and perhaps others) will also need an update to conform to the new prettified specfule template.
SPEC: http://ynemoy.fedorapeople.org/repo/ghc-X11.spec SRPM: http://ynemoy.fedorapeople.org/repo/ghc-X11-1.4.2-1.fc9.src.rpm This makes it compliant with the guidelines. This is an example of a package that dynamicaly links to C libraries and is only a library. The macros needed are currently available on the wiki and need to be installed manually for now. There is a bug to have them included in ghc directly. I will add it to the blockers. Don't forget to --copyin them to mock, and then use --no-clean to do the build.
I apologize, that's the wrong package. Here are the correct links. SPEC: http://ynemoy.fedorapeople.org/repo/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/repo/xmonad-0.7-3.fc9.src.rpm This is an example of a binary and library package.
Should xmonad not require ghc-xmonad?
Created attachment 316173 [details] xmonad.spec an updated spec file that builds with the latest macros in bug 460304
a) turns out there's a new xmonad release. I've even built packages for it. I'll test the new macros, and rebuild everything. b) xmonad generally requires ghc-xmonad. there are very very few cases where it doesn't and that's not the normal use case. since we don't have a 'highly-suggested-install-this-or-die' category like debian, i'm going to acommodate 99% of use cases and note that xmonad definitely requires ghc-xmonad (although technically the other way around isn't necessary.)
Ping?
I can update the submission but we need to get ghc-X11 reviewed first.
Let's pick this up again. SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-1.fc11.src.rpm Currently, this only compiles on rawhide, so get your rawhide builders ready.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-2.fc10.src.rpm
Created attachment 333495 [details] xmonad.spec-2.patch cabal2spec sync and some other requires tweaks. This also build, installs and uninstalls for me.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-3.fc10.src.rpm
I don't know if the ghc-X11 = 1.4.5 is really necessary? Well according to hackage it requires X11 (>=1.4.3), so I suggest using >= 1.4.3 unless there is a good reason not to. BSD3 is called BSD in Fedora. Otherwise the cabal2spec-diff looks good to me.
(Arguably the xmonad requires of ghc is redundant since it requires ghc-xmonad-devel, which requires ghc anyway, but I guess it doesn't really matter either way: I suppose it further reminds one that xmonad may recompile itself in userspace when customized.)
$ python >>> import this <snip> Explicit is better than implicit. <snip> Anyways, we need to include ghc-xmonad-devel because xmonad can and will recompile itself on the fly.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-4.fc10.src.rpm Arguably, this package needs to be rebuilt everytime there's an update to X11, because of static compilation. Still, there is one use case for giving a more generic required version of ghc-X11, and that is the user who needs to recompile to an earlier version for what ever reason. Note that fileperms was fixed too. This is an issue that seems to be common to haskell things compiled in ghc, so we may want to fix this in the template.
Thanks for the update. > Note that fileperms was fixed too. This is an issue that seems to be common to > haskell things compiled in ghc, so we may want to fix this in the template. You need to do that just for the bin file like I did with hscolour, not for all files, like this: %files %defattr(-,root,root,-) %doc LICENSE %attr(755,root,root) %{_bindir}/%{name} Otherwise looks ok to me: I would like test it in rawhide and then do the final review.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-5.fc10.src.rpm Note to self defattr != attr :P
The manpage and the example config file (xmonad.hs) from the directory "man" are not packaged. Also it seems to me that a xmonad.desktop file in /usr/share/xsessions is missing, which allows one to select xmonad as wm in gdm. I used this the following one with an older xmonad release, maybe it works with the current one, too: http://till.fedorapeople.org/files/xmonad.desktop
Created attachment 337244 [details] add desktop file, package manpage, sample config and other docs The xmonad.desktop works for me with gdm, but xmonad needs a config file in ~/.xmonad/xmonad.hs to even run. Maybe a wrapper should be added that creates such a config file if there is none. This config file could make xmonad display its manpage per default when it is started to help users new to xmonad. This was what wmii did iirc and I found it pretty helpful back then. I will write such a wrapper if it will be packaged.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-7.fc10.src.rpm This includes both a start-xmonad and an xmonad-session script. The former will check for a config file or copy a config file over. The latter will call a user's .xsession and then start start-xmonad. There are also .desktop files for both, and both should be installed in the correct location. Let me know if this works for you. Currently, the default config is the default config provided by upstream. Should we convince upstream to have it load the manpage on startup? If you still want to write a script or two, please do, and if it brings any new features / works better than mine, i will be more than happy to include them. :)
Spec file is broken: + install -p -m 0755 -D /home/bos/rpmbuild/SOURCES/start-xmonad /usr/bin/start-xmonad install: cannot create regular file `/usr/bin/start-xmonad': Permission denied error: Bad exit status from /var/tmp/rpm-tmp.sfbpdY (%install)
With that simple edit made, I get another error: Processing files: ghc-xmonad-devel-0.8.1-7.fc10 error: Could not open %files file /home/bos/rpmbuild/BUILD/xmonad-0.8.1/ghc-xmonad-devel.files: No such file or directory
I will try to test it later today, but I already found some additional issues: - There are now tabs used after Source2 to Source4 - start-xmonad probably needs a "exec xmonad" at the end to make it actually start xmonad - start-xmonad will not find the config file, because there is a doc missing in the path: /usr/share/xmonad-0.8.1/xmonad.hs, nevertheless the sample config should probably copied to /usr/share/xmonad/xmonad.hs, too and not marked as %doc, otherwise the package will not work installed with --nodocs.
BTW how about building ghc-X11 for F10 - it would make testing easier for more people I guess.
(In reply to comment #29) > This includes both a start-xmonad and an xmonad-session script. The former will > check for a config file or copy a config file over. The latter will call a > user's .xsession and then start start-xmonad. There are also .desktop files > for both, and both should be installed in the correct location. Let me know if > this works for you. I did not ttest the xsession stuff, but with my patch it works: http://till.fedorapeople.org/files/xmonad_7-8.spec.diff I wrote a simple config that is stored in /etc/skel/.xmonad/xmonad.hs, that opens the manpage using xterm. > Currently, the default config is the default config provided by upstream. > Should we convince upstream to have it load the manpage on startup? If this can be done easily within xmonad, why not. But I guess if xmonad would open the manpage by default, there would be no easy way to stop it doing this using a xmonad.hs currently.
ghc-X11 won't build on F-10, i think because of macro errors. We'll need an updated ghc for this. Meanwhile, users can just pull packages from rawhide. http://koji.fedoraproject.org/koji/taskinfo?taskID=1274046 @Till, i'll have a look at your patches later.
Yaakov built it http://koji.fedoraproject.org/koji/buildinfo?buildID=96455 unfortunately now it has higher release number than rawhide which is not good. You should use ghc-X11-1.4.5-5.fc10.1 to bump in F-10 to keep it less than ghc-X11-1.4.5-5.fc11.
Thanks for bumping the f11 ghc-X11 build, Yaakov. (In reply to comment #34) > I did not test the xsession stuff Can't it just be merged into xmonad-start? > but with my patch it works: > http://till.fedorapeople.org/files/xmonad_7-8.spec.diff Thanks > I wrote a simple config that is stored in /etc/skel/.xmonad/xmonad.hs, that > opens the manpage using xterm. Could you attach it here?
> (In reply to comment #34) > > I did not test the xsession stuff > > Can't it just be merged into xmonad-start? No, today I found a .xsession I used with an older release of xmonad I guess. It contained a exec xmonad. Btw. a different feature for xmonad-start I want to suggest is to test whether xmonad.hs is newer than the binary and in case it is, recompile it. Or is this something xmonad already does? But it seems it does not recompile itself, when I hit ALT-q. > > but with my patch it works: > > http://till.fedorapeople.org/files/xmonad_7-8.spec.diff > > Thanks > > > I wrote a simple config that is stored in /etc/skel/.xmonad/xmonad.hs, that > > opens the manpage using xterm. > > Could you attach it here? I meant to include it in the patch, otherwise the patch is not functional. I uploaded a new version of the patch to above URL, but the config file is now also available at: http://till.fedorapeople.org/files/xmonad.hs.xterm_manpage.hs
Can you post complete spec files instead of patches? It's too hard to track what's a patch to a spec file, vs a patch to the source.
(In reply to comment #39) > Can you post complete spec files instead of patches? It's too hard to track > what's a patch to a spec file, vs a patch to the source. http://till.fedorapeople.org/files/xmonad-0.8.1-8.fc10.src/
(In reply to comment #38) > Btw. a different feature for xmonad-start I want to suggest is to test whether > xmonad.hs is newer than the binary and in case it is, recompile it. Or is this > something xmonad already does? I believe it should but I am a bit rusty on xmonad. > But it seems it does not recompile itself, when > I hit ALT-q. That also work from what I was reading on their wiki. > I meant to include it in the patch, otherwise the patch is not functional. I > uploaded a new version of the patch to above URL, but the config file is now > also available Thanks - will try that out!
http://petersen.fedorapeople.org/xmonad/xmonad.spec http://petersen.fedorapeople.org/xmonad/xmonad-0.8.1-9.fc10.src.rpm I have been using this package now recently and it works ok for me - uses most of Till's latest package but slightly simpler: just xmonad.desktop for xmonad-start. I feel this is probably enough for the initial fedora xmonad package. We can get more sophisticated later if necessary, but for now I suggest advanced users who want to customize their session startup can use xorg-x11-xinit-session and ~/.xsession say.
New update SPEC: http://ynemoy.fedorapeople.org/review/xmonad.spec SRPM: http://ynemoy.fedorapeople.org/review/xmonad-0.8.1-11.fc10.src.rpm This does a few new things. 1) carries around a ppc workaround that really should go in macros.ghc 2) only builds on fedora 12 (yay?) so you need koji to make this work for you http://koji.fedoraproject.org/koji/taskinfo?taskID=1325396 3) converts the value added default config into a patch directly against the upstream source. what this patch does is replaces the xmonad line of the default verbose config with an expansion that loads xterm with the man page showing. in the future, i'll probably add a comment to the user 4) includes just xmonad-start without doing xmonad-session. 5) renumbers the source to make more sense 6) includes other changes proposed here enjoy!
(In reply to comment #43) > New update Thanks! > 1) carries around a ppc workaround that really should go in macros.ghc Yeah we could add something in the macros to help with this, but it is a bit tedious: I hope we can fix the real problem on ppc soon. > 2) only builds on fedora 12 (yay?) I hope to backport latest ghc to f11 soon. We might have to wait for a new gtk2hs release. > 3) converts the value added default config into a patch directly against the > upstream source. what this patch does is replaces the xmonad line of the > default verbose config with an expansion that loads xterm with the man page > showing. in the future, i'll probably add a comment to the user Good idea. I think better to rename the patch to xmonad-config-show-manpage.patch or something like that. > 4) includes just xmonad-start without doing xmonad-session. Good - I think the name in the .desktop file should just be "xmonad" or "XMonad" though not "xmonad-start". > 5) renumbers the source to make more sense > 6) includes other changes proposed here Ok Otherwise looks pretty good to me now. I am going to take it for a spin and then do the review.
I tested the scratch build on f11 with f12 ghc and it works fine for me. I will attach a patch for the suggestions above after this.
Here is my formal review. +:ok, !:needs attention, -:needs fixing, NA: not applicable MUST Items: [!] MUST: rpmlint output Please fix the tabs that appeared spec file for the patch. Also fixed in my next patch. [+] MUST: Package Naming Guidelines [+] MUST: spec file name must match base package %{name} [+] MUST: Packaging Guidelines. Follows Haskell Packaging Guidelines [+] MUST: Licensing Guidelines [+] MUST: License field in the package spec file must match actual license. [+] MUST: include license files in %doc if available in source [+] MUST: The spec file must be written in American English and be legible. [+] MUST: source md5sum matches upstream release 03a8f0a420902d9eea3df1d8d62598c7 xmonad-0.8.1.tar.gz [+] MUST: must successfully compile and build into binary rpms on one main arch [+] MUST: if necessary use ExcludeArch for other archs [+] MUST: All build dependencies must be listed in BuildRequires [NA] MUST: use %find_lang macro for .po translations [NA] MUST: packages which store shared library files in the dynamic linker's default paths, must call ldconfig in %post and %postun. [NA] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review [+] MUST: A package must own all directories that it creates. [+] MUST: A package must not contain any duplicate files in the %files listing. [+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. [+] MUST: Each package must have a %clean section [+] MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines. [+] MUST: The package must contain code, or permissable content. [+] MUST: Large documentation files should go in a doc subpackage. [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [+] MUST: Header files must be in a -devel package. [NA] MUST: Static libraries must be in a -static package. [NA] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). [NA] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [NA] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} [NA] MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec. [+] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: [+] SHOULD: The reviewer should test that the package builds in mock. [+] SHOULD: The package should compile and build into binary rpms on all supported architectures. [+] SHOULD: The reviewer should test that the package functions as described. [+] SHOULD: If scriptlets are used, those scriptlets must be sane. Package is APPROVED.
Created attachment 342410 [details] xmonad-final.patch I suggest making these small changes before importing to cvs.
New Package CVS Request ======================= Package Name: xmonad Short Description: a tiling window manager written in haskell Owners: ynemoy Branches: F-11 InitialCC: fedora-haskell-sig
cvs done.
Built on rawhide and Fedora 11 http://koji.fedoraproject.org/koji/buildinfo?buildID=101104 http://koji.fedoraproject.org/koji/taskinfo?taskID=1339359
Thanks, F11 build needs to be tweaked or wait for ghc-rpm-macros.
Yeah, i was gonna do that in one fell swoop. If someone could finish that review, it'll mean i won't have to bother putting in a temporary fix in the xmonad-fc11 package.
New Package CVS Request ======================= Package Name: xmonad Short Description: a tiling window manager written in haskell Owners: mathstuf Branches: el6 InitialCC: haskell-sig
Missed the flag.
You mean a package change request here right? Not a new package?
Package Change Request ======================= Package Name: xmonad New Branches: el6 Owners: mathstuf InitialCC: haskell-sig
Git done (by process-git-requests).