Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-1.20130320git6467963.fc18.src.rpm Description: Much like any other NetworkManager VPN plugin, this plugin features full VPN support via SSH tunnels for NetworkManager. Based on code from NetworkManager-openvpn. Fedora Account System Username: danfruehauf
Hi Dan, welcome to fedora, some few comments about their spec: - Please put the flag FE-NEEDSPONSOR in the field "Blocks" I think this spec is based on this http://pkgs.fedoraproject.org/cgit/NetworkManager-openvpn.git/tree/NetworkManager-openvpn.spec but even this spec does not this exempt of some errors - the use of %define is deprecated, in their instead use %global See http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define - Epoch should only be used in Fedora as a last resort to resolve upgrade ordering of a package see http://fedoraproject.org/wiki/Packaging:Guidelines#Use_of_Epochs - We have new guidelines that covering how to handle sources from Github in a Fedora Package See http://fedoraproject.org/wiki/Packaging:SourceURL#Github - Check the versions available on fedora, I'm sure that we are in the higher versions than those defined here %define nm_version 1:0.9.2 %define dbus_version 1.1 %define gtk2_version 3.0.1 %define openssh_version 6.1 %define shared_mime_version 0.16-3 if so it will not be necessary to define - not needed define %{__install} rpm -E %{__install} /usr/bin/install in make install DESTDIR=%{buildroot} INSTALL="%{__install} -p" you can call "install" directly make DESTDIR=%{buildroot} INSTALL="install -p" CP="cp -p" install - I fail to see any files .desktop (installed or listed) in /usr/share/applications/, update-desktop-database is used when a desktop entry has a 'MimeType key. See http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database - this scriptlet must be used if the application install icons into one of the subdirectories in %{_datadir}/icons/ see http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache touch --no-create %{_datadir}/icons/hicolor if [ -x /usr/bin/gtk-update-icon-cache ]; then /usr/bin/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || : fi - %defattr(-, root, root) is not needed - Why no there changelog entries ? - one question, this fork just taking whatever is in the current NetworkManager-openvpn tree or diverge? See https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries Cheers - Eduardo
Btw, the spec file are not the same of the SRPM's spec Diff spec file in url and in SRPM --------------------------------- --- /home/makerpm/923564-NetworkManager-ssh/srpm/NetworkManager-ssh.spec 2013-03-20 02:23:51.347271233 -0430 +++ /home/makerpm/923564-NetworkManager-ssh/srpm-unpacked/NetworkManager-ssh.spec 2013-03-20 02:23:51.876271264 -0430 @@ -5,11 +5,11 @@ %define shared_mime_version 0.16-3 -%define snapshot ___snapshot___ -%define realversion ___version___ +%define snapshot .20130320git6467963 +%define realversion 0.0.3 Summary: NetworkManager VPN plugin for SSH Name: NetworkManager-ssh Epoch: 1 -Version: ___version___ +Version: 0.0.3 Release: 1%{snapshot}%{?dist} License: GPLv2+ @@ -86,2 +86,290 @@ %changelog +* Fri Mar 15 2013 Dan Fruehauf +- (6467963) greatly improved rpm building: * using a proper changelog extracted from git * including a git revision tag and date * adding a dirty tag for local modifications * using a dynamic version in spec (HEAD, origin/master, origin/HEAD, master) + +* Thu Mar 7 2013 Dan Fruehauf +- (6503b7e) not building with ipv6 if no NetworkManager >= 0.9.6 found, also suggesting user to run with '--without-ipv6' + +* Wed Mar 6 2013 Dan Fruehauf +- (2238d45) Merge branch 'master' of github.com:danfruehauf/NetworkManager-ssh + +* Wed Mar 6 2013 Dan Fruehauf +- (5cfce45) Moved the 'How does it work?' section to the end as it might confuses users + +* Wed Mar 6 2013 Dan Fruehauf +- (6c7f468) waiting for QUIT signal, as specified by this bug for the openconnect plugin: https://bugzilla.gnome.org/show_bug.cgi?id=674991 - thanks for Oren Held for pointing that out + +* Mon Mar 4 2013 Dan Fruehauf +- (7ed31a7) Fixes warning on debian, thanks to Whoopie + +* Fri Mar 1 2013 Dan Fruehauf +- (4569aac) update README.md with debugging information + +* Fri Mar 1 2013 Dan Fruehauf +- (a813a7c) added import support + +* Mon Feb 25 2013 Dan Fruehauf +- (28fc866) updated screenshots + +* Mon Feb 25 2013 Dan Fruehauf +- (b07cab1) updated screenshots + +* Mon Feb 25 2013 Dan Fruehauf +- (3f7f43a) fixed build, stupid mistake + +* Mon Feb 25 2013 Dan Fruehauf +- (dcde283) added better handling of errors - canceling ifconfig timer if ssh has quitted + +* Mon Feb 25 2013 Dan Fruehauf +- (ce6c4c1) allowing to use also tap0/tun0 + +* Mon Feb 25 2013 Dan Fruehauf +- (41ffe16) fixed ipv6 support when running with --without-ipv6 + +* Mon Feb 25 2013 Dan Fruehauf +- (351368f) refactored ui file + +* Mon Feb 25 2013 Dan Fruehauf +- (d3d652f) Merge branch 'master' of github.com:danfruehauf/NetworkManager-ssh + +* Mon Feb 25 2013 Dan Fruehauf +- (8d53f80) ipv6 support optional + +* Mon Feb 25 2013 Dan Fruehauf +- (4be9c54) fixed changelogs + +* Mon Feb 25 2013 Dan Fruehauf +- (5d403e4) Merge branch 'master' of github.com:danfruehauf/NetworkManager-ssh + +* Mon Feb 25 2013 Dan Fruehauf +- (b4733f2) incremented version to 0.0.3 + +* Mon Feb 25 2013 Dan Fruehauf +- (45f2e5b) added ipv6 support + +* Sun Feb 24 2013 Dan Fruehauf +- (1dc1873) slight fix to README.md + +* Sun Feb 24 2013 Dan Fruehauf +- (e8396f4) applied Whoopie's patch for all, not just debian + +* Sun Feb 24 2013 Dan Fruehauf +- (9ed57cd) reversed Whoopie's libexec patch + +* Sun Feb 24 2013 Dan Fruehauf +- (bb9d011) updated README.md + +* Sat Feb 23 2013 Dan Fruehauf +- (13276e1) improved exporting + +* Sat Feb 23 2013 Dan Fruehauf +- (b679426) nm_ssh_get_free_device now looking properly for a free tun/tap device + +* Sat Feb 23 2013 Dan Fruehauf +- (0faf649) improved error handling from the SSH process, removed some TODOs as well + +* Sat Feb 23 2013 Dan Fruehauf +- (3e9dfed) added option to specify username (although would most likely be root) + +* Sat Feb 23 2013 Dan Fruehauf +- (3fe1e00) fixed LIBEXECDIR in nm-ssh-service.name.in, thanks to Whoopie + +* Sat Feb 23 2013 Dan Fruehauf +- (1a8b359) added export functionality for tap devices + +* Sat Feb 23 2013 Dan Fruehauf +- (5892292) added tap support + +* Sat Feb 23 2013 Dan Fruehauf +- (e439814) fixed POTFILES.in, thanks to Whoopie + +* Sat Feb 23 2013 Dan Fruehauf +- (aab4a37) improved debian build with git versioning, thanks to Whoopie + +* Wed Feb 20 2013 Dan Fruehauf +- (011a210) fixed system command when probing for tun devices + +* Tue Feb 19 2013 Dan Fruehauf +- (bd4202f) removing fds after ssh process exits, solves the problem of 100% cpu after process exits + +* Tue Feb 19 2013 Dan Fruehauf +- (7ba8924) * config variable for SSH_AUTH_SOCK * config variable for SSH_KNOWN_HOSTS_PATH + +* Tue Feb 19 2013 Dan Fruehauf +- (ee12daa) * supressing error when probing tun devices * ifconfig is now a constant + +* Tue Feb 19 2013 Dan Fruehauf +- (9375210) using user's known_hosts file instead of root's + +* Tue Feb 19 2013 Dan Fruehauf +- (817184d) added proper ignores + +* Tue Feb 19 2013 Dan Fruehauf +- (efbe6aa) added warning dialog if ssh-agent is not running + +* Mon Feb 18 2013 Dan Fruehauf +- (3a1ebe9) such mess with autoconf, added m4 again with compiler_warnings.m4 + +* Mon Feb 18 2013 Dan Fruehauf +- (7df0d99) removed even more autoconf files + +* Mon Feb 18 2013 Dan Fruehauf +- (37df930) added back Makefile.in.in in po/ + +* Mon Feb 18 2013 Dan Fruehauf +- (c6802d4) fixed autoreconf stuff + +* Sun Feb 17 2013 Dan Fruehauf +- (5998e75) updated README.md with running autoreconf + +* Sun Feb 17 2013 Dan Fruehauf +- (5112a18) removed also configure + +* Sun Feb 17 2013 Dan Fruehauf +- (9b8304c) even more cleaning of autoconf + +* Sun Feb 17 2013 Dan Fruehauf +- (98c465f) added ignores + +* Sun Feb 17 2013 Dan Fruehauf +- (9307420) removed redundant constant and reverted change of password saving in config + +* Sun Feb 17 2013 Dan Fruehauf +- (9c3711a) removed more autoconf unneeded files + +* Sun Feb 17 2013 Dan Fruehauf +- (19bd109) 0.0.2 - improved ssh-agent integration + +* Sun Feb 17 2013 Dan Fruehauf +- (6130714) added gitignore + +* Sun Feb 17 2013 Dan Fruehauf +- (e627ba9) removed autoconf redundant stuff + +* Sun Feb 17 2013 root +- (b04ecb9) greatly improved the integration with ssh-agent + +* Wed Feb 13 2013 Dan Fruehauf +- (6fd6bb4) updated README.md for deb building + +* Wed Feb 13 2013 Dan Fruehauf +- (303f182) fixed deps for ubuntu package + +* Wed Feb 13 2013 Dan Fruehauf +- (cb142e9) fixed ubuntu package building + +* Wed Feb 13 2013 Dan Fruehauf +- (0294354) fixed deps for ubuntu package + +* Wed Feb 13 2013 Dan Fruehauf +- (0ddfaa0) updated ubuntu packaging + +* Wed Feb 13 2013 Dan Fruehauf +- (c2f3c76) added ubuntu package building + +* Wed Feb 13 2013 Dan Fruehauf +- (57c6345) removed files in POTFILES.in + +* Wed Feb 13 2013 Dan Fruehauf +- (14aee09) minor changes to enable compilation on debian + +* Sat Feb 9 2013 Dan Fruehauf +- (1001eb3) got rid of some more unused files + +* Sat Feb 9 2013 Dan Fruehauf +- (b2e6f48) mistake in ChangeLog + +* Sat Feb 9 2013 Dan Fruehauf +- (e5ee901) Added screenshots + +* Sat Feb 9 2013 Dan Fruehauf +- (eab0202) removed 'show passwords' checkbox + +* Fri Feb 8 2013 Dan Fruehauf +- (c95215d) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (9fd048e) updated ChangeLog with correct date + +* Fri Feb 8 2013 Dan Fruehauf +- (d890178) fixed export of EXTRA_OPTS + +* Fri Feb 8 2013 Dan Fruehauf +- (80dc7a6) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (8cd1018) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (94a31ca) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (55df8cd) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (e9c0850) README.md updated + +* Fri Feb 8 2013 Dan Fruehauf +- (cf82af6) updated README.md + +* Fri Feb 8 2013 Dan Fruehauf +- (dbd6df8) added rpm target to Makefile.am + +* Fri Feb 8 2013 Dan Fruehauf +- (62e8def) more cleanups to building mechanism + +* Fri Feb 8 2013 Dan Fruehauf +- (91cd028) changed README and spec to conform with github + +* Fri Feb 8 2013 Dan Fruehauf +- (2dc862c) Initial first commit + +* Fri Feb 8 2013 Dan Fruehauf +- (446bd32) Initial commit + +* Fri Feb 8 2013 Dan Fruehauf +- (16355cc) added spec for fedora/redhat + +* Fri Feb 8 2013 Dan Fruehauf +- (50c9904) fixed resolving function + +* Fri Feb 8 2013 Dan Fruehauf +- (49e2b61) added resolving and some more cleanups + +* Fri Feb 8 2013 Dan Fruehauf +- (eafedd6) removed common directory and fixed parsing of extra_ops + +* Fri Feb 8 2013 Dan Fruehauf +- (f25525b) more cleanups, most of the stuff is in configuration + +* Fri Feb 8 2013 Dan Fruehauf +- (227348f) using remote tun number from configuration + +* Fri Feb 8 2013 Dan Fruehauf +- (b9529b0) got rid of common directory + +* Fri Feb 8 2013 Dan Fruehauf +- (3142b9e) heaps more changes, things are actually working now + +* Fri Feb 8 2013 Dan Fruehauf +- (6fdffb0) more cleanups and GUI starts to look much better + +* Wed Feb 6 2013 Dan Fruehauf +- (9fd3ef1) * modified AUTHORS * changed get_ssh_agent_socket to static + +* Wed Feb 6 2013 Dan Fruehauf +- (fabeac2) * fixed makefiles with gio-unix * fixed copyright + +* Wed Feb 6 2013 Dan Fruehauf +- (dc429de) modified configure.ac + +* Wed Feb 6 2013 Dan Fruehauf +- (0ff6a77) probing ssh-agent socket + +* Tue Feb 5 2013 Dan Fruehauf +- (d9f2d75) cleanups, removed nm-ssh-service-ssh-helper + +* Mon Feb 4 2013 Dan Fruehauf +- (1984e3d) nm-ssh: first commit imported from nm-openvpn
Hi, First and foremost thank you. I'm happy to be part of Fedora. And thank you for the prompt and detailed response. I'll relate to the points you've mentioned one by one. - Added FE-NEEDSPONSOR - Using now %global instead of %define - Removed Epoch tag - I'll have a look shortly at the guidelines for github projects, I was actually looking for something like that - Added a TODO for myself to remove these lines if they are unnecessary - Changed the install call to what you suggested: make DESTDIR=%{buildroot} INSTALL="install -p" CP="cp -p" install - No .desktop files, it merely installs a NetworkManager plugin - No installing any icons as far as I'm concerned - Removed the %defattr line - I'm generating the Changelog dynamically from github commits, is that OK? - I'm based on the code in NetworkManager-openvpn, however by that time the code is totally different and serves a totally different purpose. There will always be similarity between the various VPN plugins as they implement the same interface, but solves a different problem. - The .spec I've attached is being handled by a make target (make rpm) and does the following: - Replaces version in .spec - Replaces github version (revision) in .spec - Generates the Changelog - I'm pretty sure after I'll read the github guidelines I'll be smarter about all of that. Thanks, Dan.
Hi (In reply to comment #3) I'm generating the Changelog dynamically from github commits, is that OK? No, the changelog are related to the spec not to the code changes, see https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs, please place the changelog by hand %define snapshot ___snapshot___ %define realversion ___version___ Version: ___version___ Release: 1%{snapshot}%{?dist} and equally either the snapshot or version must be placed by hand, see http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages > - The .spec I've attached is being handled by a make target (make rpm) and > does the following: > - Replaces version in .spec > - Replaces github version (revision) in .spec > - Generates the Changelog Although this is fine for generating rpms on demand, not right for a valid rpm in Fedora repositories - Please add COPYING file, in this moment not listed in %doc - Please, see https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Convincing_someone_to_sponsor_you, to search a willing sponsor, especially try doing some informal reviews to other packagers. - Remember check the available versions of gtk3-devel, dbus-devel, NetworkManager-devel, NetworkManager-glib-devel, if the versions are greater than which you define, the declarations >= are not necessary ( Example: echevemaster@echevemaster ~$ repoquery -qf --whatprovides --releasever=18 NetworkManager-devel NetworkManager-devel-1:0.9.7.0-12.git20121004.fc18.i686 NetworkManager-devel-1:0.9.7.0-12.git20121004.fc18.x86_64 NetworkManager-devel-1:0.9.8.0-1.fc18.i686 NetworkManager-devel-1:0.9.8.0-1.fc18.x86_64 NetworkManager-devel-1:0.9.7.0-12.git20121004.fc18.i686 NetworkManager-devel-1:0.9.7.0-12.git20121004.fc18.x86_64 * NetworkManager-devel-1:0.9.8.0-1.fc18.i686 * NetworkManager-devel-1:0.9.8.0-1.fc18.x86_64 in your case the system-wide version is greater than you which you define %define nm_version 1:0.9.2
Oh, i'm sorry, i had a copy n' paste error (in the case of NetworkManager-devel) , this not applied to the dependent packages with epoch, which must be declared anywhere as %{epoch}:%{version}-%{release}
Hey again, - All versions were greater, so they were removed. Not depending on these versions anymore. - Fixed release tag according to: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag http://fedoraproject.org/wiki/Packaging:SourceURL#Github - Added COPYING file - Removed ChangeLog file from RPM package as a full changelog is in the spec - Decided it'll be a pre release as things are still in development over there Files are here for your reinspection: Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-pre1.20130321gitcf6c00f.fc18.src.rpm Appreciated.
Hi again Dan I go back on my words, the changelog is not related to the code changes of the application, is related with the changes in the spec, by example, a good changelog would look like this * Wed Mar 20 2013 Dan Fruehauf <malkodan> - 0.0.0-2 - Added COPYING file - Removed ChangeLog file from RPM package as a full changelog is in the spec * Wed Mar 20 2013 Dan Fruehauf <malkodan> - 0.0.0-1 - Initial packaging Remember bump the release number, every time you make a change to spec. As you can see only in the changelog, only there entries about the modifications made to spec - Afaik, Github not generate xz files, only zip or tar.gz. https://github.com/danfruehauf/NetworkManager-ssh/archive/cf6c00f117a761c8b0e88462c84e99df58c4a352/NetworkManager-ssh-0.0.3-cf6c00f.tar.xz gives an error 404 not so: https://github.com/danfruehauf/NetworkManager-ssh/archive/cf6c00f117a761c8b0e88462c84e99df58c4a352/NetworkManager-ssh-0.0.3-cf6c00f.tar.gz or https://github.com/danfruehauf/NetworkManager-ssh/archive/cf6c00f117a761c8b0e88462c84e99df58c4a352/NetworkManager-ssh-0.0.3-cf6c00f.zip you should also download the file, preserving the timestamps, with the command wget -N or curl -R. see http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps this scriptlets: %post /usr/bin/update-desktop-database > /dev/null touch --no-create %{_datadir}/icons/hicolor if [ -x /usr/bin/gtk-update-icon-cache ]; then /usr/bin/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || : fi %postun /usr/bin/update-desktop-database > /dev/null touch --no-create %{_datadir}/icons/hicolor if [ -x /usr/bin/gtk-update-icon-cache ]; then /usr/bin/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || : fi not needed, no there .desktop files or icons listed in your spec, equally is applicable to Requires, for the same reason Requires(post): %{_bindir}/update-desktop-database Requires(postun): %{_bindir}/update-desktop-database - Please the date of the snapshot should be placed by hand - the naming is incorrect, in the case of pre-release package you should follow this guidelines http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages - Please, remember my comment in the case of NetworkManager-devel, or any package that makes use of epoch (In reply to comment #5) > Oh, i'm sorry, i had a copy n' paste error (in the case of > NetworkManager-devel) , this not applied to the dependent packages with > epoch, which must be declared anywhere as %{epoch}:%{version}-%{release} See http://fedoraproject.org/wiki/Packaging:Guidelines#Use_of_Epochs I quote: "Also, Epoch complicates normal packaging guidelines. If a package uses an Epoch, it must be referred to in any place where %{version}-%{release} is used. For example, if a package being depended upon has an Epoch, this must be listed when adding a versioned dependency: Requires: foo = %{epoch}:%{version}-%{release}. " Cheers
Hi Eduardo, Seems almost like I manage to get one step forward and three steps back :) - Fixed changelog in spec file, including only changes to spec (packaging wise) - Building RPM with .gz and not .xz - Verified the github URL is valid (https://github.com/danfruehauf/NetworkManager-ssh/archive/8767415030da697a1a08cb536166c0ca7bb037b2/NetworkManager-ssh-0.0.3-8767415.tar.gz) - Remove scriptlets - If there's nothing in %post and $postun, should the sections exist anyway? - %{checkout} is defined by hand and conforming what I could understand in the URL you've provided - I've changed the %{checkout} tag to conform the standards, although it's not 100% clear. An example in that file would make things heaps clearer... - What is exactly %{alphatag}? Is it defined at all? %{X}? - Currently I have 'Release: 2.%{checkout}%{?dist}' - Release (2, or %{X}) is separated by a comma from %{checkout} - %{checkout} is 20130322git%{shortcommit} - much like '20110102git9e88d7e' in the URL - But I'm not using epoch anyway anymore anywhere! No versioned dependencies I've posted a new .spec and SRPM: Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-2.20130322git8767415.fc18.src.rpm I really hope we're making some sort of progress, I'm really trying my best as you can see... :) Many thanks, Dan.
Hi Dan, I'm sorry for delay, I was busy in this days. (In reply to comment #8) > - Building RPM with .gz and not .xz > - Verified the github URL is valid > (https://github.com/danfruehauf/NetworkManager-ssh/archive/ > 8767415030da697a1a08cb536166c0ca7bb037b2/NetworkManager-ssh-0.0.3-8767415. > tar.gz) I have a question about this, and I'll summarize in the following points: 1.- If you download the tarball in SOURCES folder and extracted the files of this tarball the resultant folder should be NetworkManager-ssh-8767415030da697a1a08cb536166c0ca7bb037b2, Why the tarball in the SRPM is NetworkManager-ssh-0.0.3? the tarball should not be modified, that means that the sources must be pristine, should be packaged as they are, the only way in which it can be modified is via patch. 2.- %setup in this case should be %setup -q -n %{name}-%{commit} 3.- the sources in github they have no file autogen.sh or configure, therefore, mock fails to build. check it with mock or fedora-review > - Remove scriptlets > - If there's nothing in %post and $postun, should the sections exist > anyway? %post and %postun must be removed, Requires(post): %{_bindir}/update-desktop-database Requires(postun): %{_bindir}/update-desktop-database also must be removed > - I've changed the %{checkout} tag to conform the standards, although it's > not 100% clear. An example in that file would make things heaps clearer... > - What is exactly %{alphatag}? Is it defined at all? %{X}? > - Currently I have 'Release: 2.%{checkout}%{?dist}' > - Release (2, or %{X}) is separated by a comma from %{checkout} > - %{checkout} is 20130322git%{shortcommit} - much like > '20110102git9e88d7e' in the URL a good example is this link: http://pkgs.fedoraproject.org/cgit/python-docutils.git/tree/python-docutils.spec Release Tag for Pre-Release Packages: 0.%{X}.%{alphatag} Where %{X} is the release number increment, and %{alphatag} is the string that came from the version i.e. 0.2.20130322git8767415 => 0.%{X}.%{alphatag} also in the changelog Fri Mar 22 2013 Dan Fruehauf <malkodan> - 0.0.3-0.2.20130322git8767415 - Changes to conform with Fedora packaging standards > I really hope we're making some sort of progress, I'm really trying my best > as you can see... :) Of course we are progressing, after we have reviewed the package thoroughly, You will be sponsored ;) Regards Eduardo -
(In reply to comment #9) > Hi Dan, I'm sorry for delay, I was busy in this days. > > (In reply to comment #8) > > > - Building RPM with .gz and not .xz > > - Verified the github URL is valid > > (https://github.com/danfruehauf/NetworkManager-ssh/archive/ > > 8767415030da697a1a08cb536166c0ca7bb037b2/NetworkManager-ssh-0.0.3-8767415. > > tar.gz) > I have a question about this, and I'll summarize in the following points: > 1.- If you download the tarball in SOURCES folder and extracted the files > of this tarball the resultant folder should be > NetworkManager-ssh-8767415030da697a1a08cb536166c0ca7bb037b2, Why the tarball > in the SRPM is NetworkManager-ssh-0.0.3? the tarball should not be modified, > that means that the sources must be pristine, should be packaged as they > are, the only way in which it can be modified is via patch. > Taking a tarball from the github URL specified - fixed. > 2.- %setup in this case should be %setup -q -n %{name}-%{commit} > Fixed and works (tested build from src.rpm). > 3.- the sources in github they have no file autogen.sh or configure, > therefore, mock fails to build. check it with mock or fedora-review > Changed to autoreconf -fvi Added 'BuildRequires: autoconf' > > - Remove scriptlets > > - If there's nothing in %post and $postun, should the sections exist > > anyway? > %post and %postun must be removed, > Requires(post): %{_bindir}/update-desktop-database > Requires(postun): %{_bindir}/update-desktop-database > also must be removed Removed, fixed. > > > > - I've changed the %{checkout} tag to conform the standards, although it's > > not 100% clear. An example in that file would make things heaps clearer... > > - What is exactly %{alphatag}? Is it defined at all? %{X}? > > - Currently I have 'Release: 2.%{checkout}%{?dist}' > > - Release (2, or %{X}) is separated by a comma from %{checkout} > > - %{checkout} is 20130322git%{shortcommit} - much like > > '20110102git9e88d7e' in the URL > > a good example is this link: > > http://pkgs.fedoraproject.org/cgit/python-docutils.git/tree/python-docutils. > spec > Followed that link and now 'Release: 0.3.%{checkout}%{?dist}' So fixed as well. > Release Tag for Pre-Release Packages: 0.%{X}.%{alphatag} > Where %{X} is the release number increment, and %{alphatag} is the string > that came from the version > > i.e. > > 0.2.20130322git8767415 => 0.%{X}.%{alphatag} > also in the changelog > Fixed. > Fri Mar 22 2013 Dan Fruehauf <malkodan> - > 0.0.3-0.2.20130322git8767415 > - Changes to conform with Fedora packaging standards > Fixed. > > I really hope we're making some sort of progress, I'm really trying my best > > as you can see... :) > Of course we are progressing, after we have reviewed the package thoroughly, > You will be sponsored ;) > > Regards > Eduardo - Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-0.3.20130326gite27a6ae.fc18.src.rpm Thanks, Dan.
Hi Dan - not built in rawhide, g_type_init is deprecated in glib 2.35.4, the latest release in rawhide is glib2-2.36.0-1.fc20, please patch this, adding a conditional to built correctly nm-ssh-service.c:1552:2: error: 'g_type_init' is deprecated (declared at /usr/in clude/glib-2.0/gobject/gtype.h:669) [-Werror=deprecated-declarations] g_type_init (); ^ cc1: all warnings being treated as errors make[2]: *** [nm-ssh-service.o] Error 1 make[2]: Leaving directory `/builddir/build/BUILD/NetworkManager-ssh-e27a6ae109f 0c1f715ee7bf6ae5abd8aff8c7480/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/builddir/build/BUILD/NetworkManager-ssh-e27a6ae109f0c1f715ee7bf6ae5abd8aff8c7480' make: *** [all] Error 2 - I had not seen this carefully %{_datadir}/gnome-vpn-properties/ssh/nm-ssh-dialog.ui %dir %{_datadir}/gnome-vpn-properties/ssh this must be %dir %{_datadir}/gnome-vpn-properties/ssh %{_datadir}/gnome-vpn-properties/ssh/nm-ssh-dialog.ui -rpmlint output Rpmlint ------- Checking: NetworkManager-ssh-0.0.3-0.3.20130326gite27a6ae.fc18.x86_64.rpm NetworkManager-ssh.x86_64: W: spelling-error %description -l en_US capabilites -> capabilities, capability, liabilities NetworkManager-ssh.x86_64: W: incoherent-version-in-changelog 0.0.3-0.3.20130326git7549f1db1b ['0.0.3-0.3.20130326gite27a6ae.fc18', '0.0.3-0.3.20130326gite27a6ae'] Please double-check the changelog NetworkManager-ssh.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/nm-ssh-service.conf This should be marked as %config, probably, %config(noreplace) NetworkManager-ssh.x86_64: W: non-conffile-in-etc /etc/NetworkManager/VPN/nm-ssh-service.name This should be marked as %config, probably, %config(noreplace) 1 packages and 0 specfiles checked; 0 errors, 4 warnings. - You provide this patch %{_datadir}/gnome-vpn-properties/ssh, echevemaster@echevemaster ~$ rpm -qf /usr/share/gnome-vpn-properties/ nm-connection-editor-0.9.8.0-1.fc18.x86_64 but %{_datadir}/gnome-vpn-properties is ownership (only f18) of nm-connection-editor, in previous releases is of NetworkManager-gnome, add the following conditional %if 0%{?fedora} > 17 Requires: nm-connection-editor %else Requires: NetworkManager-gnome %endif
(In reply to comment #11) > Hi Dan > > - not built in rawhide, g_type_init is deprecated in glib 2.35.4, the latest > release in rawhide is glib2-2.36.0-1.fc20, please patch this, adding a > conditional to built correctly > > nm-ssh-service.c:1552:2: error: 'g_type_init' is deprecated (declared at > /usr/in > clude/glib-2.0/gobject/gtype.h:669) [-Werror=deprecated-declarations] > g_type_init (); > ^ > cc1: all warnings being treated as errors > make[2]: *** [nm-ssh-service.o] Error 1 > make[2]: Leaving directory > `/builddir/build/BUILD/NetworkManager-ssh-e27a6ae109f > 0c1f715ee7bf6ae5abd8aff8c7480/src' > make[1]: *** [all-recursive] Error 1 > make[1]: Leaving directory > `/builddir/build/BUILD/NetworkManager-ssh- > e27a6ae109f0c1f715ee7bf6ae5abd8aff8c7480' > make: *** [all] Error 2 > Fixed (guarding with an #ifdef around g_type_init()) Generally speaking, any Rawhide compiling machines I can use? > - I had not seen this carefully > %{_datadir}/gnome-vpn-properties/ssh/nm-ssh-dialog.ui > %dir %{_datadir}/gnome-vpn-properties/ssh > this must be > %dir %{_datadir}/gnome-vpn-properties/ssh > %{_datadir}/gnome-vpn-properties/ssh/nm-ssh-dialog.ui > Fixed (it was the order, yes?) > -rpmlint output > > Rpmlint > ------- > Checking: NetworkManager-ssh-0.0.3-0.3.20130326gite27a6ae.fc18.x86_64.rpm > NetworkManager-ssh.x86_64: W: spelling-error %description -l en_US > capabilites -> capabilities, capability, liabilities Fixed the typo. > NetworkManager-ssh.x86_64: W: incoherent-version-in-changelog > 0.0.3-0.3.20130326git7549f1db1b ['0.0.3-0.3.20130326gite27a6ae.fc18', > '0.0.3-0.3.20130326gite27a6ae'] > > Please double-check the changelog > > NetworkManager-ssh.x86_64: W: non-conffile-in-etc > /etc/dbus-1/system.d/nm-ssh-service.conf > This should be marked as %config, probably, %config(noreplace) > Fixed. > NetworkManager-ssh.x86_64: W: non-conffile-in-etc > /etc/NetworkManager/VPN/nm-ssh-service.name > This should be marked as %config, probably, %config(noreplace) > 1 packages and 0 specfiles checked; 0 errors, 4 warnings. > Fixed. [dan@ugi fedora-rpms]$ rpmlint /home/dan/rpmbuild/RPMS/x86_64/NetworkManager-ssh-0.0.3-0.4.20130328git1af74fd.fc18.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. > - You provide this patch > %{_datadir}/gnome-vpn-properties/ssh, Which patch? > > echevemaster@echevemaster ~$ rpm -qf /usr/share/gnome-vpn-properties/ > nm-connection-editor-0.9.8.0-1.fc18.x86_64 > > but %{_datadir}/gnome-vpn-properties > is ownership (only f18) of nm-connection-editor, in previous releases is of > NetworkManager-gnome, add the following conditional > > %if 0%{?fedora} > 17 > Requires: nm-connection-editor > %else > Requires: NetworkManager-gnome > %endif Fixed. A bit of a confusion about the changelog in the spec. Say I have revision X which is last for the changelog, however revision Y, which is newer is the version I want to build, if I build revision Y and the last entry is revision X, rpmlint complains about it. Hence I assume that in the changelog, the last entry should be of the commit I currently build (which doesn't make any sense, because it's not the last commit the spec was changed), correct me if I'm wrong. So I've included an entry of: '* Thu Mar 28 2013 Dan Fruehauf <malkodan> - 0.0.3-0.4.%{checkout}' Any other entry will result in a rpmlint yielding an error. Please clarify that issue for me. Speaking of which, if you've found so many errors here, are you aware of the status of NetworkManager and all of the rest of its plugins? Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-0.4.20130328git1af74fd.fc18.src.rpm Thanks, Dan.
Just realized that NetworkManager-ssh doesn't work on Fedora 18 because of the dbus policy issues discussed on NetworkManager mailing list. Is it any better on branched Fedora 19 or rawhide? If not, you should probably start a bug report about that on NetworkManager and make your review request bug report depend on it.
Pavel, True, yes. I provided the patch a couple of days ago. You were responding to that as well. From my personal perspective I really don't mind if it'll make it only in FC19 or anything. The link is here: https://mail.gnome.org/archives/networkmanager-list/2013-March/msg00108.html I can attach the patch here which could be very easily back ported. I'm very much a newbie when it comes to Fedora releases, so I'll let you guys call the shots and I'll follow with whatever is decided. As you can see I do my best to make things work the Fedora way. Thanks, Dan.
(In reply to comment #14) > Pavel, > > True, yes. I provided the patch a couple of days ago. You were responding to > that as well. Yes, you provided it upstream and the Fedora maintainers are upstream developers. > From my personal perspective I really don't mind if it'll make it only in > FC19 or anything. That is why I think it would be the best way to start the Fedora NetworkManager bug report. That way you can ask the maintainers to include that patch or a release/snapshot of NM in rawhide and possibly in F19 and even F18. That way you give them a chance to respond. > The link is here: > https://mail.gnome.org/archives/networkmanager-list/2013-March/msg00108.html > > I can attach the patch here which could be very easily back ported. > > I'm very much a newbie when it comes to Fedora releases, so I'll let you > guys call the shots and I'll follow with whatever is decided. This is not how it works. You, as the future maintainer, decide which releases you target with your package. This bug report is not suitable for thorough discussions of what should be done with NetworkManager itself, as it's a bug for NetworkManager-ssh. That's why a new bug report should be IMO started unless you already know the policy works for you in the releases. > As you can see I do my best to make things work the Fedora way. Then I believe the Fedora way is what I described above :).
(In reply to comment #15) > (In reply to comment #14) > > Pavel, > > > > True, yes. I provided the patch a couple of days ago. You were responding to > > that as well. > > Yes, you provided it upstream and the Fedora maintainers are upstream > developers. > > > From my personal perspective I really don't mind if it'll make it only in > > FC19 or anything. > > That is why I think it would be the best way to start the Fedora > NetworkManager bug report. That way you can ask the maintainers to include > that patch or a release/snapshot of NM in rawhide and possibly in F19 and > even F18. > > That way you give them a chance to respond. > > > The link is here: > > https://mail.gnome.org/archives/networkmanager-list/2013-March/msg00108.html > > > > I can attach the patch here which could be very easily back ported. > > > > I'm very much a newbie when it comes to Fedora releases, so I'll let you > > guys call the shots and I'll follow with whatever is decided. > > This is not how it works. You, as the future maintainer, decide which > releases you target with your package. This bug report is not suitable for > thorough discussions of what should be done with NetworkManager itself, as > it's a bug for NetworkManager-ssh. That's why a new bug report should be IMO > started unless you already know the policy works for you in the releases. Thanks for letting me take responsibility. As you can see I'm trying to get advice from the experienced people here :) If so, I'll try to target FC18, as I think the NetworkManager patch is tiny and could be implemented easily (it's not even a code change). So say I want to open a bug report, what should be the topic? Any standard for the topic? 'NetworkManager-ssh dbus support in NetworkManager' with a target of FC18? Then should I attach the suggested patch and hope that the maintainer will include it in the next build of NetworkManager on FC18? Should I make this bug depend on the bug I'm going to open? > > > As you can see I do my best to make things work the Fedora way. > > Then I believe the Fedora way is what I described above :). Many thanks Pavel. :)
Hi Dan. I think the best way we can proceed is to first bring this package to the devel branch, aka rawhide or f19, and then open the bug against f18, You can always ask a package change request, requesting a new branch About the latest comments, regarding the package: (In reply to comment #12) > Fixed (it was the order, yes?) Indeed > > - You provide this patch > > %{_datadir}/gnome-vpn-properties/ssh, > Which patch? I'm sorry for the typo s/patch/path, ;) > A bit of a confusion about the changelog in the spec. > Say I have revision X which is last for the changelog, however revision Y, > which is newer is the version I want to build, if I build revision Y and the > last entry is revision X, rpmlint complains about it. > Hence I assume that in the changelog, the last entry should be of the commit > I currently build (which doesn't make any sense, because it's not the last > commit the spec was changed), correct me if I'm wrong. > So I've included an entry of: > '* Thu Mar 28 2013 Dan Fruehauf <malkodan> - 0.0.3-0.4.%{checkout}' > Any other entry will result in a rpmlint yielding an error. Please clarify > that issue for me. Let's go to the list of commits https://github.com/danfruehauf/NetworkManager-ssh/commits/master the two latest commits are 1af74fd6251eb031dc33753d35c230b98a34ec21 and ccf99d29a55b3090603c6266906142bc513de1c0 The fedora package maintainers, must try and as far as possible to package the latest version of the package which owns. So, if you have something like this %global commit 1af74fd6251eb031dc33753d35c230b98a34ec21 %global shortcommit %(c=%{commit}; echo ${c:0:7}) %global checkout 20130328git%{shortcommit} the changelog should be * Thu Mar 28 2013 Dan Fruehauf <malkodan> - 0.0.3-0.4.20130328git1af74fd - Fixed more issues in spec to conform with Fedora Packaging standards respecting the snapshot taken. Now in fact, the last commit you have in your git is ccf99d29a55b3090603c6266906142bc513de1c0 so your spec should look like this %global commit ccf99d29a55b3090603c6266906142bc513de1c %global shortcommit %(c=%{commit}; echo ${c:0:7}) %global checkout 20130329git%{shortcommit} and the changelog * Thu Mar 29 2013 Dan Fruehauf <malkodan> - 0.0.3-0.5.20130329gitccf99d2 - New comment about the changes. Btw, althought 0.0.3-0.4.%{checkout} is an elegant solution, is not allowed because in the changelog, macros are not acceptable check it rpmlint -iv NetworkManager-ssh-0.0.3-0.4.20130328git1af74fd.fc20.src.rpm NetworkManager-ssh.src: I: checking NetworkManager-ssh.src: I: checking-url https://github.com/danfruehauf/NetworkManager-ssh (timeout 10 seconds) NetworkManager-ssh.src:67: W: macro-in-%changelog %{checkout} Macros are expanded in %changelog too, which can in unfortunate cases lead to the package not building at all, or other subtle unexpected conditions that affect the build. Even when that doesn't happen, the expansion results in possibly "rewriting history" on subsequent package revisions and generally odd entries eg. in source rpms, which is rarely wanted. Avoid use of macros in %changelog altogether, or use two '%'s to escape them, like '%%foo'. NetworkManager-ssh.src: I: checking-url https://github.com/danfruehauf/NetworkManager-ssh/archive/1af74fd6251eb031dc33753d35c230b98a34ec21/NetworkManager-ssh-0.0.3-1af74fd.tar.gz (timeout 10 seconds) 1 packages and 0 specfiles checked; 0 errors, 1 warnings. > Speaking of which, if you've found so many errors here, are you aware of the > status of NetworkManager and all of the rest of its plugins? Yes, I am aware, so I invite you to open new bugs against these packages that have these issues, once you are maintainer (non limiting) even you can do right now if you want. Let me know what is your decision on the branch you're going to take initially, outside the small changes that I suggest in this comment, I see the package acceptable for a formal review Cheers
(In reply to comment #17) > Hi Dan. > > I think the best way we can proceed is to first bring this package to the > devel branch, aka rawhide or f19, and then open the bug against f18, You can > always ask a package change request, requesting a new branch > > About the latest comments, regarding the package: > > (In reply to comment #12) > > Fixed (it was the order, yes?) > > Indeed > > > > - You provide this patch > > > %{_datadir}/gnome-vpn-properties/ssh, > > Which patch? > I'm sorry for the typo s/patch/path, ;) > > > > A bit of a confusion about the changelog in the spec. > > Say I have revision X which is last for the changelog, however revision Y, > > which is newer is the version I want to build, if I build revision Y and the > > last entry is revision X, rpmlint complains about it. > > Hence I assume that in the changelog, the last entry should be of the commit > > I currently build (which doesn't make any sense, because it's not the last > > commit the spec was changed), correct me if I'm wrong. > > So I've included an entry of: > > '* Thu Mar 28 2013 Dan Fruehauf <malkodan> - 0.0.3-0.4.%{checkout}' > > Any other entry will result in a rpmlint yielding an error. Please clarify > > that issue for me. > > Let's go to the list of commits > https://github.com/danfruehauf/NetworkManager-ssh/commits/master > the two latest commits are > 1af74fd6251eb031dc33753d35c230b98a34ec21 > and > ccf99d29a55b3090603c6266906142bc513de1c0 > > The fedora package maintainers, must try and as far as possible to package > the latest version of the package which owns. > > So, if you have something like this > %global commit 1af74fd6251eb031dc33753d35c230b98a34ec21 > %global shortcommit %(c=%{commit}; echo ${c:0:7}) > %global checkout 20130328git%{shortcommit} > > the changelog should be > * Thu Mar 28 2013 Dan Fruehauf <malkodan> - > 0.0.3-0.4.20130328git1af74fd > - Fixed more issues in spec to conform with Fedora Packaging standards > > respecting the snapshot taken. > > Now in fact, the last commit you have in your git is > ccf99d29a55b3090603c6266906142bc513de1c0 > so your spec should look like this > > %global commit ccf99d29a55b3090603c6266906142bc513de1c > %global shortcommit %(c=%{commit}; echo ${c:0:7}) > %global checkout 20130329git%{shortcommit} > > and the changelog > > * Thu Mar 29 2013 Dan Fruehauf <malkodan> - > 0.0.3-0.5.20130329gitccf99d2 > - New comment about the changes. It's a bit like a chicken and egg. I'll just prepare a correct spec with the current date and the last revision. > > Btw, althought 0.0.3-0.4.%{checkout} is an elegant solution, is not allowed > because in the changelog, macros are not acceptable > > check it > > rpmlint -iv NetworkManager-ssh-0.0.3-0.4.20130328git1af74fd.fc20.src.rpm > NetworkManager-ssh.src: I: checking > NetworkManager-ssh.src: I: checking-url > https://github.com/danfruehauf/NetworkManager-ssh (timeout 10 seconds) > NetworkManager-ssh.src:67: W: macro-in-%changelog %{checkout} > Macros are expanded in %changelog too, which can in unfortunate cases lead to > the package not building at all, or other subtle unexpected conditions that > affect the build. Even when that doesn't happen, the expansion results in > possibly "rewriting history" on subsequent package revisions and generally > odd > entries eg. in source rpms, which is rarely wanted. Avoid use of macros in > %changelog altogether, or use two '%'s to escape them, like '%%foo'. > > NetworkManager-ssh.src: I: checking-url > https://github.com/danfruehauf/NetworkManager-ssh/archive/ > 1af74fd6251eb031dc33753d35c230b98a34ec21/NetworkManager-ssh-0.0.3-1af74fd. > tar.gz (timeout 10 seconds) > 1 packages and 0 specfiles checked; 0 errors, 1 warnings. > New src.rpm is good to go: [dan@ugi fedora-rpms]$ rpmlint -vi NetworkManager-ssh-0.0.3-0.5.20130330git0fe4747.fc18.src.rpm NetworkManager-ssh.src: I: checking NetworkManager-ssh.src: I: checking-url https://github.com/danfruehauf/NetworkManager-ssh (timeout 10 seconds) NetworkManager-ssh.src: I: checking-url https://github.com/danfruehauf/NetworkManager-ssh/archive/0fe47471442629954484cb128b1b610cf0576a68/NetworkManager-ssh-0.0.3-0fe4747.tar.gz (timeout 10 seconds) 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Current spec file is spotless!! :) > > > Speaking of which, if you've found so many errors here, are you aware of the > > status of NetworkManager and all of the rest of its plugins? > > Yes, I am aware, so I invite you to open new bugs against these packages > that have these issues, once you are maintainer (non limiting) even you can > do right now if you want. I might actually do it and provide patches since I know what has to be done more or less. I'll talk to the guys on #nm. > > Let me know what is your decision on the branch you're going to take > initially, outside the small changes that I suggest in this comment, I see > the package acceptable for a formal review > Lets go rawhide then anyway. Hopefully we'll be quick enough to get it in to FC19 before its release? > Cheers Cheers again! Here we go, hopefully last iteration: Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-0.5.20130330git0fe4747.fc18.src.rpm
(In reply to comment #16) > If so, I'll try to target FC18, as I think the NetworkManager patch is tiny > and could be implemented easily (it's not even a code change). That's a good idea. Then we need to apply the fix for f18, f19 and rawhide. > So say I want to open a bug report, what should be the topic? Any standard > for the topic? > 'NetworkManager-ssh dbus support in NetworkManager' with a target of FC18? Exactly. It should be implied but it may be better to specify that you are targetting to f18, f19 and rawhide. > Then should I attach the suggested patch and hope that the maintainer will > include it in the next build of NetworkManager on FC18? Link to the commit is good enough: http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=edd1ebe8a0f6703f7863684566011e34a9715186 > Should I make this bug depend on the bug I'm going to open? Yes. > Many thanks Pavel. :) I'm happy there's someone willing to extend the networking ecosystem :). (In reply to comment #17) > Hi Dan. > > I think the best way we can proceed is to first bring this package to the > devel branch, I disagree. > aka rawhide or f19, and then open the bug against f18, You can > always ask a package change request, requesting a new branch This increases the bureaucratic burden but doesn't solve anything. It's as easy to ask for patching rawhide NetworkManager as to ask for patching all branches. > > A bit of a confusion about the changelog in the spec. > > Say I have revision X which is last for the changelog, however revision Y, > > which is newer is the version I want to build, if I build revision Y and the > > last entry is revision X, rpmlint complains about it. > > Hence I assume that in the changelog, the last entry should be of the commit > > I currently build (which doesn't make any sense, because it's not the last > > commit the spec was changed), correct me if I'm wrong. > > So I've included an entry of: > > '* Thu Mar 28 2013 Dan Fruehauf <malkodan> - 0.0.3-0.4.%{checkout}' > > Any other entry will result in a rpmlint yielding an error. Please clarify > > that issue for me. When you bump your version/release, you should also add a changelog record. The 'rpmdev-bumpspec' from @fedora-packager will do that for you. You don't even need to stuff everything in one commit. It is your choice which commit you use for building. It will work in the same way with fedora git repositories. Also please don't use your projects git branch for tracking of the fedora spec file. > The fedora package maintainers, must try and as far as possible to package > the latest version of the package which owns. This is not necessarily true. Often it's the latest released version. Often it's one of known-working git snapshots. > Btw, althought 0.0.3-0.4.%{checkout} is an elegant solution, is not allowed > because in the changelog, macros are not acceptable +1 You must have literal versions in your changelog or otherwise it wouldn't work as a changelog. > > Speaking of which, if you've found so many errors here, are you aware of the > > status of NetworkManager and all of the rest of its plugins? > > Let me know what is your decision on the branch you're going to take > initially, outside the small changes that I suggest in this comment, I see > the package acceptable for a formal review +1 (In reply to comment #18) > It's a bit like a chicken and egg. > I'll just prepare a correct spec with the current date and the last revision. Don't use your projects' source control to track the Fedora specfile and you'll be fine. > I might actually do it and provide patches since I know what has to be done > more or less. I'll talk to the guys on #nm. You can submit the patches upstream to NetworkManager bugzilla or NetworkManager mailing list. If you find problems with the Fedora packages themselves (problems in specfile, etc), use this bugzilla and the respective package names. > > Let me know what is your decision on the branch you're going to take > > initially, outside the small changes that I suggest in this comment, I see > > the package acceptable for a formal review > > > Lets go rawhide then anyway. Hopefully we'll be quick enough to get it in to > FC19 before its release? You can do f18/f19/rawhide at once and be done with it. > Spec URL: http://nm-ssh.cloudapp.net/NetworkManager-ssh.spec > SRPM URL: > http://nm-ssh.cloudapp.net/NetworkManager-ssh-0.0.3-0.5.20130330git0fe4747. > fc18.src.rpm Thanks!
Eduardo, I thought I already assigned myself earlier and was going to do the review. But if you had the same idea and want to take it yourself, just let me know and I'll transfer the bug to you. Cheers, Pavel
(In reply to comment #20) > Eduardo, > > I thought I already assigned myself earlier and was going to do the review. > But if you had the same idea and want to take it yourself, just let me know > and I'll transfer the bug to you. > > Cheers, > > Pavel Yes. Pavel I had the same idea ;). Please tranfer the bug to me, I'll sponsor to Dan. ---------------- Dan, I think it would be great idea that once approved the package, add to Pavel and me as co-maintainers. I'm sure we can help you throughout the process
Lifting FE-NEEDSPONSOR, I'll do the review today, but later, because Bugzilla intermittently for maintenance Dan, you are already sponsored, congratulations and welcome to package maintainers group
Package Review ============== Key: [x] = Pass [!] = Fail [-] = Not applicable [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. No, not in the LDPATH [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: Package complies to the Packaging Guidelines [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL (v2 or later)". 1 files have unknown license. Detailed output of licensecheck in /home/makerpm/nm31/923564-NetworkManager- ssh/licensecheck.txt [x]: The spec file handles locales properly. [x]: Package consistently uses macro is (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [-]: Large documentation must go in a -doc subpackage. Note: Documentation size is 40960 bytes in 4 files. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: %config files are marked noreplace or the reason is justified. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [-]: Fully versioned dependency in subpackages, if present. [x]: Spec file lacks Packager, Vendor, PreReq tags. [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: No %config files under /usr. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: No rpmlint messages. ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Uses parallel make. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define. ===== EXTRA items ===== Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: NetworkManager-ssh-0.0.3-0.5.20130330git0fe4747.fc20.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint NetworkManager-ssh 1 packages and 0 specfiles checked; 0 errors, 0 warnings. # echo 'rpmlint-done:' Requires -------- NetworkManager-ssh (rpmlib, GLIBC filtered): NetworkManager config(NetworkManager-ssh) dbus gnome-keyring gtk3 libatk-1.0.so.0()(64bit) libc.so.6()(64bit) libcairo-gobject.so.2()(64bit) libcairo.so.2()(64bit) libdbus-1.so.3()(64bit) libdbus-glib-1.so.2()(64bit) libgdk-3.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libgio-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgnome-keyring.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgtk-3.so.0()(64bit) libnm-glib-vpn.so.1()(64bit) libnm-glib.so.4()(64bit) libnm-util.so.2()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) libpthread.so.0()(64bit) nm-connection-editor openssh rtld(GNU_HASH) shared-mime-info Provides -------- NetworkManager-ssh: NetworkManager-ssh NetworkManager-ssh(x86-64) config(NetworkManager-ssh) libnm-ssh-properties.so()(64bit) Unversioned so-files -------------------- NetworkManager-ssh: /usr/lib64/NetworkManager/libnm-ssh-properties.so MD5-sum check ------------- https://github.com/danfruehauf/NetworkManager-ssh/archive/0fe47471442629954484cb128b1b610cf0576a68/NetworkManager-ssh-0.0.3-0fe4747.tar.gz : CHECKSUM(SHA256) this package : 37ec0ec49555da9c67df37cd33164d71bc101404fffec0e52fe00595ace162a4 CHECKSUM(SHA256) upstream package : 37ec0ec49555da9c67df37cd33164d71bc101404fffec0e52fe00595ace162a4 Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 923564 -m fedora-rawhide-x86_64 ---------------- PACKAGE APPROVED ---------------- Dan, you can now proceed to request a repository: https://fedoraproject.org/wiki/Package_SCM_admin_requests If you have any questions , feel free to contact me through my email or in the irc channel #fedora-devel, my nick is echevemaster
Guys, thank you very much!! Happy to be a part of Fedora!! About the issues at stake: - Open a bug for NetworkManager-ssh and possibly backporting dbus NetworkManager patch to fc18 and fc19 - I'm very new with the Fedora packaging system, I think I'll contact Eduardo on IRC and further understand what I should do (how to set you as co-maintainers for instance) - Pavel, I'll probably continue managing the spec with the same git, however I'm much smarter about it now, so it shouldn't impose any problems. Other than that, I'm excited and hopefully it'll boost me to also maintain more packages in Fedora once I get NetworkManager-ssh to a desired state. BR
New Package SCM Request ======================= Package Name: NetworkManager-ssh Short Description: NetworkManager VPN plugin for SSH Owners: danfruehauf echevemaster pavlix Branches: f18 f19 InitialCC:
Pavel, Just wrote the mailing list, apparently the patch I've submitted it not needed. I've just verified it. So I guess we can continue without opening another bug. However I already posted in the mailing list about that and we might just remove the whole section about allowing per-vpn-type. But that's another but. Altogether I think we can proceed with the build?
(In reply to comment #24) > Guys, thank you very much!! Happy to be a part of Fedora!! Thanks Eduardo for doing the review. > About the issues at stake: > - Open a bug for NetworkManager-ssh and possibly backporting dbus > NetworkManager patch to fc18 and fc19 > - I'm very new with the Fedora packaging system, I think I'll contact > Eduardo on IRC and further understand what I should do (how to set you as > co-maintainers for instance) For other packages, I'm just requesting commit access and the owner grants it to me. But I don't know the precise meaning of co-maintainership either. > - Pavel, I'll probably continue managing the spec with the same git, This is not how Fedora works. But you will learn that anyway. > however I'm much smarter about it now, so it shouldn't impose any problems. > > Other than that, I'm excited and hopefully it'll boost me to also maintain > more packages in Fedora once I get NetworkManager-ssh to a desired state. Apart from submitting new packages, you can also read through my packages and request commit access to those you would like to help with and post me a mail about what help I can expect :). https://admin.fedoraproject.org/pkgdb/users/packages/pavlix > BR (In reply to comment #26) > Just wrote the mailing list, apparently the patch I've submitted it not > needed. Your package didn't work on my Fedora 18 because of dbus policy issues. So either you fixed the issue in NetworkManager-ssh, or you're getting a bug report on NetworkManager-ssh once you push the build. I see you are going to support NetworkManager-ssh on Fedora 18 (and thanks for that), so we should resolve the issue in a way or other. > I've just verified it. So I guess we can continue without opening another > bug. However I already posted in the mailing list about that and we might > just remove the whole section about allowing per-vpn-type. But that's > another but. > > Altogether I think we can proceed with the build? Sure. Make builds. Test them. Fix the policy issues. But you don't need to push updates to branches, yet, and can wait until the issues are resolved, AFAIK.
Git done (by process-git-requests).
Pavel, You've followed up in the NetworkManager mailing list, however I want to make sure the issue is clear. Even if we patch NetworkManager - yes or no, we'll still have to suggest the user to restart NetworkManager. If the user will have to restart NetworkManager, do we really want the patch? In future versions if the patch is not applied and the NetworkManager-ssh package is downloaded after NetworkManager and the patch was not applied, a restart will be needed. If the patch would be applied - it wouldn't require a restart. If someone does not perform a distribution upgrade (f18 for instance) and updates NetworkManager with the patch and downloads NetworkManager-ssh - things will not work until a restart of NetworkManager. In my opinion the patch should be applied regardless, since all the known VPN plugins are recognized in NetworkManager dbus config file, however much of a difference it doesn't make since the current limitations will require a restart in some cases. It wouldn't be "clean". Please advise what you think is the best solution.
(In reply to comment #29) > Pavel, > > You've followed up in the NetworkManager mailing list, however I want to > make sure the issue is clear. > > Even if we patch NetworkManager - yes or no, we'll still have to suggest the > user to restart NetworkManager. If the user will have to restart > NetworkManager, do we really want the patch? Sure we do. It fixes the problem at least with the restart. > In future versions if the patch is not applied and the NetworkManager-ssh > package is downloaded after NetworkManager and the patch was not applied, a > restart will be needed. If the patch would be applied - it wouldn't require > a restart. Please elaborate. This way you're only adding more confusion. Please be more precise. It looks like your contradicting yourself blatantly which suggests there's some misunderstanding. > If someone does not perform a distribution upgrade (f18 for instance) and > updates NetworkManager with the patch and downloads NetworkManager-ssh - > things will not work until a restart of NetworkManager. > > In my opinion the patch should be applied regardless, since all the known > VPN plugins are recognized in NetworkManager dbus config file, however much > of a difference it doesn't make since the current limitations will require a > restart in some cases. It wouldn't be "clean". From the current information I have, we're choosing between: a) It will work after restart b) It will not work at all Corrent me if I'm wrong. And also correct the information you provided. As in this situation I'm of course for #a as it will allow me to use the package, while #b won't. > Please advise what you think is the best solution. I'm afraid that if you are even thinking about not applying the patch, then there must be some confusion that you should clarify first. I can't imagine why would you want to deliver a package that doesn't work.
(In reply to comment #30) > (In reply to comment #29) > > Pavel, > > > > You've followed up in the NetworkManager mailing list, however I want to > > make sure the issue is clear. > > > > Even if we patch NetworkManager - yes or no, we'll still have to suggest the > > user to restart NetworkManager. If the user will have to restart > > NetworkManager, do we really want the patch? > > Sure we do. It fixes the problem at least with the restart. > Not exactly. > > In future versions if the patch is not applied and the NetworkManager-ssh > > package is downloaded after NetworkManager and the patch was not applied, a > > restart will be needed. If the patch would be applied - it wouldn't require > > a restart. > > Please elaborate. This way you're only adding more confusion. Please be more > precise. It looks like your contradicting yourself blatantly which suggests > there's some misunderstanding. > > > If someone does not perform a distribution upgrade (f18 for instance) and > > updates NetworkManager with the patch and downloads NetworkManager-ssh - > > things will not work until a restart of NetworkManager. > > > > In my opinion the patch should be applied regardless, since all the known > > VPN plugins are recognized in NetworkManager dbus config file, however much > > of a difference it doesn't make since the current limitations will require a > > restart in some cases. It wouldn't be "clean". > > From the current information I have, we're choosing between: > > a) It will work after restart > b) It will not work at all > > Corrent me if I'm wrong. And also correct the information you provided. As > in this situation I'm of course for #a as it will allow me to use the > package, while #b won't. > > > Please advise what you think is the best solution. > > I'm afraid that if you are even thinking about not applying the patch, then > there must be some confusion that you should clarify first. I can't imagine > why would you want to deliver a package that doesn't work. In terms of dbus policy, every NetworkManager VPN plugin ships: /etc/dbus-1/system.d/nm-X-service.conf In our case: /etc/dbus-1/system.d/nm-ssh-service.conf These files include the line: --- <allow send_destination="org.freedesktop.NetworkManager.ssh"/> --- However these lines are also in /etc/dbus-1/system.d/org.freedesktop.NetworkManager.conf - which ships with NetworkManager: --- <allow send_destination="org.freedesktop.NetworkManager.ssh"/> --- So every plugin theoretically takes care of itself. However if you install a new plugin, its policy (in /etc/dbus-1/system.d/nm-X-service.conf) will not be used until NetworkManager is restarted, this is the dbus bug we were referring to. Having this line in the NetworkManager dbus config (/etc/dbus-1/system.d/org.freedesktop.NetworkManager.conf) as a dup for that just means that if NetworkManager is running and you add a new VPN plugin - it'll work without a restart (because it loaded the config allowing all VPN plugins). I hope this time I've made more sense... :)
(In reply to comment #31) > I hope this time I've made more sense... :) It makes sense, theoretically. But then please let me know when I can actually successfully try to use NetworkManager-ssh and whether there's something special I have to do.
So the patch made it into NetworkManager 0.9.8.1, which is in updates-testing. Eduardo, should I change the spec to depend on NetworkManager 0.9.8.1 and send a build request? Pavel, even after updating to NetworkManager-0.9.8.1, it'll still require a restart of NetworkManager for things to work.
(In reply to comment #33) > So the patch made it into NetworkManager 0.9.8.1, which is in > updates-testing. > > Eduardo, should I change the spec to depend on NetworkManager 0.9.8.1 and > send a build request? No, build the package for now in rawhide if you prefer. when nm pass to stable, then build in the other branches
FYI Just build for Rawhide. Will build for f18 and f19 when NetworkManager-0.9.8.1 goes into stable.
Please see: https://bugzilla.redhat.com/show_bug.cgi?id=863836#c16
Pavel, sounds good. much like in debian. I'll work shortly on separating it into a NetworkManager-ssh and NetworkManager-ssh-gnome plugin. Shouldn't be too difficult.
(In reply to comment #37) > Pavel, sounds good. much like in debian. I'll work shortly on separating it > into a NetworkManager-ssh and NetworkManager-ssh-gnome plugin. Shouldn't be > too difficult. Thanks. Subpackaging is actually very easy. Consult the packaging guidelines and/or ask me or Eduardo.
It's divided now to sub-packages: Spec URL: http://nm-ssh.cloudapp.net/nm-sub-packages/NetworkManager-ssh.spec SRPM URL: http://nm-ssh.cloudapp.net/nm-sub-packages/NetworkManager-ssh-0.0.3-0.7.20130405git3d5321b.fc18.src.rpm Eduardo/Pavel, I'll be more than happy to get feedback on that. The mock build looks good though (rpmlint has no errors on spec or build packages). And at the same time, since I've been through the process of making NetworkManager-ssh.spec standard, I'll be happy to do the same for the rest of the NetworkManager VPN plugins.
(In reply to comment #39) > It's divided now to sub-packages: > Spec URL: http://nm-ssh.cloudapp.net/nm-sub-packages/NetworkManager-ssh.spec > SRPM URL: > http://nm-ssh.cloudapp.net/nm-sub-packages/NetworkManager-ssh-0.0.3-0.7. > 20130405git3d5321b.fc18.src.rpm The package review is finished with positive result and you have been granted git access. It's time to start using the Fedora git repository for your package and the various Fedora infrastructure tools like koji and bodhi. You don't have to be afraid of that, when you build a package using koji, it's up to you whether you push it as an update through bodhi or whether you keep it just for testing. > Eduardo/Pavel, I'll be more than happy to get feedback on that. The mock > build looks good though (rpmlint has no errors on spec or build packages). > > And at the same time, since I've been through the process of making > NetworkManager-ssh.spec standard, I'll be happy to do the same for the rest > of the NetworkManager VPN plugins. This is as easy as checking out their git repositories (e.g. using fedpkg clone <package-name>), modifying the spec file, producing a patch (e.g. using git diff) and start a bug report with that patch. Cheers, Pavel
NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc19
NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc18
NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc19 has been pushed to the Fedora 19 testing repository.
NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc19 has been pushed to the Fedora 19 stable repository.
NetworkManager-ssh-0.0.3-0.8.20130419git3d5321b.fc18 has been pushed to the Fedora 18 stable repository.