Initial comments, full review after hearing your thoughts on these:

Subpackage naming: how about smart-gtk and smart-tui?  "tui" is used eg. by many
system-config-* packages for their text UI's, and plain "gtk" sounds better than
"uigtk" to me.  Further, what benefit is there in splitting the text UI from the
main package in the first place, does it add some dependencies or...?

How about packaging contrib/ksmarttray?  (%package -n ksmarttray)

Some 3rd party packagers package contrib/smart-update too, would that be useful?

I agree with changing the GUI subpackage to -gtk, but what benefit does having
the text-mode UI in a separate subpackage give?

(In reply to comment #2)
> what benefit does having
> the text-mode UI in a separate subpackage give?

Yep, I asked that too in comment 1 :)

Any chance of naming the package "smartpm" (a la "smartpm.org")? This is very
confusingly close to the SMART technology used in hard drives (smartmontools
package)....

- merged -uitext into main package and added the corresponding Provides:
  there
- renamed -uigtk to -gtk
- added 'smart-fedora-setup' and execute it in the %post section


http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.4.src.rpm

============


* ok, text interface is now provided by the main package
  (comment #1 + comment #2)

* how can I build ksmarttray? afais, the tarballs lacks the needed
  'configure' and 'Makefile.in' files.
  (comment #1)

* at first glance 'smart-update' smells like a suid program whose
  functionality can be replaced by 'sudo' entirely. It seems to miss
  important features like propagating of $http_proxy environment
  variables, too. Because it is not build + installed by default I
  will not ship it.
  (comment #1)

* I would like 'smartpm' as the packagename too and the spec file is
  prepared for the the 'smartpm' packagename But FE packaging guidelines
  require package-name == tarball-name.
  (comment #4)

Here's one specfile that builds ksmarttray:
http://dag.wieers.com/packages/smart/smart.spec

Dag's specfile could be worth looking into for other reasons too, for example
the distro.py kernel handling stuff (which could/should be extended to other
than UP and SMP kernels as well, eg. xen0, xenU, kdump, xen-hypervisor,
xen-guest, maybe various kernel-devel packages and later Extras kernel module
packages, and maybe RHEL-specific kernel variants), unless your package already
takes care of it someway.

Also, some channels would be nice to have out of the box.  Maybe
smart-fedora-setup already takes care of that; I haven't had a look yet.

ok, I added a -ksmarttray subpackage. I am not sure whether I should
make it an own package ('-n ksmarttray').


smart-fedora-setup adds a special handling for default kernels but
your additional variants are missing. Is it possible to enumerate
all affected packages? Handling of the new, virtual 'kernel-module'
provides will need upstream changes, too.


I submitted a 'smart-channels' package (bug #175473) for review which
contains definitions for Fedora channels. Although it has probably the
same effect like a plain 'Requires:' (which would be wrong), I added a
dependency on it with a 'Requires(missingok): smart-channels'.

All other packagers I'm aware of ship ksmarttray as "ksmarttray", not
"smart-ksmarttray", I'd suggest following that.  It just looks and feels better
that way IMO (bundling in the tarball aside, kyum vs yum-kyum, yumex vs
yum-yumex, synaptic vs apt-synaptic etc).

I don't know of a way to enumerate kernel variants but I don't think listing a
bunch of known ones would be much of a maintenance burden.  It wouldn't work for
custom kernels though.  If it helps, kernel module packages can also be
recognized by a name prefix or something in the future once the proposal is
finalized.

What rpm versions does Requires(missingok) work with?  (At least so that the
specfile parser groks it.)

Specifying "tui" or "gtk" as the version to ui(%name) sounds strange, and now
that the text UI is bundled in the main package, ui(%name) doesn't seem to serve
a purpose any more.

desktop-file-install warns:
/home/scop/rpmbuild/SOURCES/smart.desktop: missing encoding  (guessed UTF-8)

ksmarttray build fails on FC5t1+rawhide, configure tries to use -lXt but it
isn't found.  BuildRequires: libXt-devel fixes it.

How is ksmarttray supposed to be found by end users?  Some kind of menu entry
(an usual one, or one that is listed in KDE's "add applet" choices) should be
included.

I don't think sudo is the correct tool to handle updating with ksmarttray,
something with the ability to prompt for the root password in a GUI would sound
better (eg.  consolehelper and friends).

IMO smart-gtk should be changed to use consolehelper too.

If you want to have a Provides: smartpm, adding Provides: smartpm-gtk to the gtk
subpackage too wouldn't hurt.

> eg.  consolehelper and friends 
 
Or kdesu, given that it's a KDE package. 

* Tue Dec 13 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.6 
- use %%python_sitelib instead of %%_libdir/... 
- added Encoding entry in desktop file 
- removed the ui(...) virtual provides for now 
- made 'ksmarttray' an real package (instead of a subpackage) and 
  added desktop entry 
- added -usermode subpackage which contains consolehelper wrappers for 
  smart and use it in the -gtk subpackage 
- enhanced 'smart-update' script 

http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.6.src.rpm

=====================


> I don't know of a way to enumerate kernel variants but I don't
> think listing a bunch of known ones would be much of a maintenance
> burden. It wouldn't work for custom kernels though.  If it helps,
> kernel module packages can also be recognized by a name prefix or
> something in the future once the proposal is finalized.

ok, I added xen0 + xenU

But the 'multiple-version' flag seems to be a match-whole-string
thing. So there must be done something upstream.


> What rpm versions does Requires(missingok) work with?  (At least so
> that the specfile parser groks it.)

Should not matter. 'smart' itself requires at least rpm 4.4 and this
version understands '(missingok)'.


> Specifying "tui" or "gtk" as the version to ui(%name) sounds strange,
> and now that the text UI is bundled in the main package, ui(%name)
> doesn't seem to serve a purpose any more.

ok, removed for now


> ksmarttray build fails on FC5t1+rawhide, configure tries to use -lXt
> but it isn't found.  BuildRequires: libXt-devel fixes it.

looks like a kdelibs-devel bug. I can not fix it now because I can not
express this BuildRequires: for pre-FC5 rpms.

When kdelibs-devel is not fixed till approval, I will add this
buildrequires for the FC5 branch in the CVS. For now, I will assume
builds on FC4.


> How is ksmarttray supposed to be found by end users?  Some kind of
> menu entry (an usual one, or one that is listed in KDE's "add applet"
> choices) should be included.

I added a ksmarttray.desktop but do not know how to test it


> I don't think sudo is the correct tool to handle updating with
> ksmarttray, something with the ability to prompt for the root
> password in a GUI would sound better (eg.  consolehelper and
> friends).

I think, sudo is the correct tool. 'ksmarttray' seems to execute
'smart-update' regularly and I would dislike periodically appearing
dialog boxes which are asking for a root password.


> IMO smart-gtk should be changed to use consolehelper too.

ok, added -usermode subpackage which provides the corresponding
wrappers.

(In reply to comment #10)
> - use %%python_sitelib instead of %%_libdir/... 

This will most likely break on x86_64.  I have no way to test it, but I'm pretty
sure that you should be using %python_sitearch instead (in fedora-rpmdevtools
python spec template terms).


> - added -usermode subpackage which contains consolehelper wrappers for 
>   smart and use it in the -gtk subpackage 

I wonder what benefit does having this in a subpackage have over just including
it in the main package or the -gtk subpackage?


> ok, I added xen0 + xenU

Please add xen-hypervisor and xen-guest too, that's the way they'll be called in
FC5.  Also, "kernel-devel" is not enough, there are -devel packages for all of
the variants (kernel-smp-devel etc).  Oh, and one typo: s/bigmen/bigmem/ (and
IIRC there's a hugemem kernel in RHEL too).

 
> > What rpm versions does Requires(missingok) work with?  (At least so
> > that the specfile parser groks it.)
> 
> Should not matter. 'smart' itself requires at least rpm 4.4 and this
> version understands '(missingok)'.

Wouldn't it be appropriate to add the corresponding versioned "BuildRequires:
rpm-build >= 4.4" and "Requires: rpm-python >= 4.4" dependencies, then?


> > ksmarttray build fails on FC5t1+rawhide, configure tries to use -lXt
> > but it isn't found.  BuildRequires: libXt-devel fixes it.
> 
> looks like a kdelibs-devel bug.

I'm not so sure about that, see contrib/ksmarttray/admin/acinclude.m4.in.  Many
configure scripts tend to test for X presence using Xt, even if they don't
actually use Xt themselves for anything else.


> When kdelibs-devel is not fixed till approval, I will add this
> buildrequires for the FC5 branch in the CVS. For now, I will assume
> builds on FC4.

Ack.

 
> I added a ksmarttray.desktop but do not know how to test it

I'll try it out and report back.


> I think, sudo is the correct tool. 'ksmarttray' seems to execute
> 'smart-update' regularly and I would dislike periodically appearing
> dialog boxes which are asking for a root password.

Good point.  OTOH consolehelper could be used to launch ksmarttray itself, but
I'm not 100% sure if that's the best approach either.  I won't consider this a
blocker (but others are of course free to disagree, though).


One new finding: -gtk has superfluous Requires(pre) and Requires(postun) for the
main package, probably a remainder from the ui(%name) and separate text UI
package times.

*** Bug 175630 has been marked as a duplicate of this bug. ***

I accidentially submitted smart for inclusion, too (see duplicate above). I
think most parts are similar, Enrico, maybe you'd like to check anyway.

One thing though: python_sitelib or python_archlib? smart installs so files
under the python subdir.

* Tue Dec 13 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.7
- use %%python_sitearch instead of %%python_sitelib
- added yet more variants for the multi-version kernel packages


==========



> > - use %%python_sitelib instead of %%_libdir/... 
> 
> This will most likely break on x86_64.  I have no way to test it, but
> I'm pretty sure that you should be using %python_sitearch instead (in
> fedora-rpmdevtools python spec template terms).

ok, I use sitearch now but can not test it


> > - added -usermode subpackage which contains consolehelper wrappers for 
> >   smart and use it in the -gtk subpackage 
> 
> I wonder what benefit does having this in a subpackage have over
> just including it in the main package or the -gtk subpackage?

* -usermode is not required for core-functionality
* -usermode adds non-trivial deps

==> -usermode can not be in the core package


* -usermode can be used for the text-interface too
* -gtk is not required for the text-interface

==> -usermode and -gtk should not be in the same package


> Wouldn't it be appropriate to add the corresponding versioned "BuildRequires:
> rpm-build >= 4.4" and "Requires: rpm-python >= 4.4" dependencies, then?

IMO, such versioned dependencies are pretty useless in the age of
missing-epoch == epoch-zero.  rpm does not provide a way to express a
dependency on a certain upstream version (e.g. 'rpm = 1:4.0' would
fulfill the Requires: but would not have the needed functionality).

Versioned deps can be used to exclude certain packages in a well-known
environment. And this well-known environment is FC4+ in this case
which is known to have the required 'rpm' package.


> One new finding: -gtk has superfluous Requires(pre) and
> Requires(postun) for the main package, probably a remainder from the
> ui(%name) and separate text UI package times.

It's intentionally. I want to make sure that 'smart-gtk' gets
installed after 'smart' and removed before it, because -gtk uses
directories from the base package which would stay when 'smart' is
removed before 'smart-gtk'.

afaik, pre/postun modifiers are the only way *guaranting* that.

URL to latest so far: http://ensc.de/fedora/smart-0.40-1.7.src.rpm

(In reply to comment #14)

> * -usermode adds non-trivial deps

Hmm, could you elaborate what these are?  glib2, libattr, pam, libselinux and
libuser sound like things that are pretty hard to get rid of in a functional FC
setup.
 
> And this well-known environment is FC4+ in this case
> which is known to have the required 'rpm' package.

Fine with me.  rpm doesn't have an epoch so that problem doesn't currently exist
in FC/RHEL, but the versioned deps would help people who try to rebuild this for
earlier distro versions or RHEL.  (Some kind of support for those is sort of
implied elsewhere in the package, eg. smart-fedora-setup.)

> I want to make sure that 'smart-gtk' gets
> installed after 'smart' and removed before it, because -gtk uses
> directories from the base package which would stay when 'smart' is
> removed before 'smart-gtk'.

Plain requires is fine for install time.  And at install time it wouldn't matter
anyway as long as all dirs are owned.

> afaik, pre/postun modifiers are the only way *guaranting* that.

Even if postun may work with the erase ordering, I wouldn't count on that abuse
(adding a dependency for a %postun script which doesn't exist).  The only way to
properly fix this is in rpm.  It isn't known to hurt though, so if you insist on
keeping it, not a blocker.

smart-fedora-setup doesn't appear to do anything useful when run as non-root,
maybe move it to /usr/sbin?

See also bug 175630 comment 2.

* Wed Dec 14 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.8
- moved smart-fedora-setup to %sbindir

http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.8.src.rpm


==================


> > * -usermode adds non-trivial deps
> Hmm, could you elaborate what these are?  glib2, libattr, pam,
> libselinux and libuser sound like things that are pretty hard to
> get rid of in a functional FC setup.

'usermode' itself; I do not see much sense for 'libuser' on plain
server machines neither and glib2 is a thorn in my side too

None of my machines has SELinux enabled so I would have nothing against
dropping the libselinux dep ;)


> the versioned deps would help people who try to rebuild this for
> earlier distro versions or RHEL.  (Some kind of support for those is
> sort of implied elsewhere in the package, eg. smart-fedora-setup.)

mmh... I blindly copied the 'installonlypkgs' list from 'yum' into
'smart-fedora-setup'. It was never my intention to support earlier
distro versions or RHEL... ;)


> > I want to make sure that 'smart-gtk' gets installed after 'smart'
> > and removed before it, because -gtk uses directories from the
> > base package which would stay when 'smart' is removed before
> > 'smart-gtk'.
> Plain requires is fine for install time.  And at install time it
> wouldn't matter anyway as long as all dirs are owned.

can you guarantee that there will not come a future package introducing
a circular dependency between smart and smart-gtk?


> > afaik, pre/postun modifiers are the only way *guaranting* that.
> Even if postun may work with the erase ordering, I wouldn't count on
> that abuse (adding a dependency for a %postun script which doesn't
> exist).

When you want to survive with rpm you have to do such abuse or hacks
;)


> smart-fedora-setup doesn't appear to do anything useful when run as
> non-root, maybe move it to /usr/sbin?

done

(In reply to comment #16)
> can you guarantee that there will not come a future package introducing
> a circular dependency between smart and smart-gtk?

Even if such a package would somehow appear, it would have to invoke something
that requires both smart and smart-gtk in its %pre or %post scriptlets and
manage to get itself inserted in a transaction before both of them are installed
for it to matter at install time.  Or are you thinking about some other
scenario?  Of course I cannot make any guarantees, but I do promise to fix the
problem if it appears ;)

To summarize my remaining wishlist and IMO's, I think I can be persuaded to let
them pass but I'd like to hear feedback from other reviewers:
- -usermode should be folded into the main package
- add >= 4.4 versions to rpm-python and rpm-build dependencies
- Requires(pre/postun) abuses should be pruned (at least the (pre) part)

Comments on bug 175630 comment 2 (why separate package for channels) is also
unaddressed so far, but I don't have strong opinions on that.

Will start actually testing the package today :)

Oh, I see you just commented on the separate channels package, thanks.

o I'd keep usermod in the main package. Reason is that end-users will not really
  know they want it, until they see it, and it doesn't cost much.
o Keep versioned BuildRequires (but I think this should be >= 4.3)
o pre/postun tricks to cure rpm oredring issues is an overkill. We'd have to do 
  that for every package then, and it was meaning something else once. Better to
  attack this in rpm itself.

I'd also like to address compatibilty to the given user base. Dag and ATrpms
users have been using smart for over a year now, and therefore it would seem
appropriate to properly obsolete/provide bits from there to ensure non-breakage
of operation.

(Hey, this would be the first package in cooperative mode :)

> o I'd keep usermod in the main package. Reason is that end-users will
>   not really know they want it, until they see it, and it doesn't cost
>   much.

end-users who need such help are usually installing 'smart-gtk' which
brings -usermod with it.

users with the text interface only will probably prefer to execute
'smart' as root or through sudo. 'consolehelper' on CLI does not offer
very much.

I just checked all my vservers and none of them has 'usermode'
installed.  Because I do not trust Fedora Core package qualitity
very much (who knows whether the next 'usermode' adds a perl- or
mysql-dependency ;) <shudder>) I would prefer to reduce additional
dependencies as far as possible.

An additional package does not hurt, but additional deps do.



> o pre/postun tricks to cure rpm oredring issues is an overkill. We'd
>   have to do that for every package then,

I do this for my packages... ;) I gave up the 'Requires(pre): filesystem'
some time ago ;) but I want to make sure that non-trivial and not-owned-by-me 
directories are in place before I put files into them.


> I'd also like to address compatibilty to the given user base. Dag
> and ATrpms users have been using smart for over a year now, and
> therefore it would seem appropriate to properly obsolete/provide
> bits from there to ensure non-breakage of operation.

ok, I will check that

%post of the main package still calls /usr/bin/smart-fedora-setup (wrong path)

* Thu Dec 15 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.9
- fixed path of smart-fedora-setup in %%post


http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.9.src.rpm

Created attachment 122275 [details]
ksmarttray desktop entry improvements

Created attachment 122276 [details]
fix ksmarttray icon install path

Created attachment 122277 [details]
icon/menu entry improvements, cosmetics

- Install icons to %_datadir/icons/hicolor.
- Improve ksmarttray desktop entry, make icon themeable and visible in
  GNOME too.

AFAIK GNOME can nowadays use KDE applets just fine.  GNOME desktop on my
rawhide box is currently so borked that I'm unable to do anything with it, so
can't test right now though.

thx, applied the patches:

* Thu Dec 15 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.10
- obsolete 'smart-gui' + 'smart-update' packages from Dag and ATrpms
  as suggested in bz #175438 comment 19

* Thu Dec 15 2005 Ville Skyttä <ville.skytta at iki.fi>
- Install icons to %_datadir/icons/hicolor.
- Improve ksmarttray desktop entry, make icon themeable and visible in
  GNOME too.


http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.10.src.rpm

GNU Arch:
| $ tla register-archive ensc http://ensc.de/tla/{archives}/fedora
| $ tla get -A ensc smart--review--0 smart




========


This version obsoletes some packages from DAG and ATrpms. Currently, it
is a simple 'Obsoletes:'. Shall I also add a corresponding 'Provides:'?

Please make the obsoletes always versioned otherwise this will harm
compatibility instead of supporting it. And also use provides in general, I
can't say whether any other packages depend on obsoleted items (I can for
ATrpms, but not for all other repos).

I didn't check with Guideline wording, but the suggestion there should be to use
(simplified w/o potential epochs):

Obsolete: old <= %{version}-%{release}
Provide: old = %{version}-%{release}

(In reply to comment #27)
> Obsolete: old <= %{version}-%{release}
> Provide: old = %{version}-%{release}

Nitpicks of the day: that would be self-obsoletion, fixed with s/<=/</, and it's
often seen a good practice to not let version of Obsoletes move along with the
package's V-R, but to hardcode known explicit numbers there instead.

* Thu Dec 15 2005 Enrico Scholz <enrico.scholz.de> - 0.40-1.11
- made the Obsoletes: versioned and added the corresponding Provides:

http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.40-1.11.src.rpm

I'd rather see smart-gui than smart-gtk as the GUI package name
- it describes the functionality of the package instead of what toolkit it
happens to use for a particular purpose
- smart-gui is already in use by Dag & Axel

If it some day gains a separate KDE interface then by all means split it to
smart-gui-gtk and smart-gui-kde or similar.

FYI: today's Rawhide metadata (or something) seems to put smart into some kind
of loop in the "Saving cache..." phase.  I don't know if it ever finishes but it
apparently consumes all available memory here, and I can't really "wait and see"
ATM...

0.41 is out, should contain a faster rpm-md parser.

I see you've updated to 0.41, any particular reason why it's not posted here yet?

Ping?  If you don't have time for this, I can take over the submission.

* Sun Dec 25 2005 Enrico Scholz <enrico.scholz.de> - 0.41-0
- updated to 0.41

http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.41-0.1.src.rpm

=================

sorry, I have some patches where is some disagreement about them and
where I would like some upstream feedback from Gustavo. But ok, try
the release above...


@comment #30
============

-uigtk + -uitext subpackages were my initial idea and they would meet
your 'smart-gui-gtk' suggestion in some way. But people did not liked
this naming so it was reduced to plain '-gtk'...

GUI is not GUI (gtk apps do not work fine in non-gnome environments),
so I like it when the used toolkit is visible in the package name.

I see there's an applet for KDE. Where's smart-gsmarttray?

About package names... -tui is usually used for text uis and -gui for graphical
uis. Perhaps you should do a separate subpackage for the text ui as smart-tui..?
(If the text ui is something the user can live without. If I've understood
correctly, it is.) I don't know which is better for the gui package: smart-gui,
smart-gui-gtk or smart-gtk. I'd say choose the name which follows other similar
package names. (I think smart-gui would follow better. At least there are
system-config-*-gui packages out there but no system-config-*-gtk packages.)

Actually I should have said "Where's gsmarttray?". Anyway, I've always liked the
up2date system tray (because it lets me run the updates manually but still tells
me when there are new updates) and I'd be happy if smart could have a similar
applet. But, I'd like to see gnome/gtk style applet in addition to the kde/qt
one because I won't be installing qt and kde libraries just because of one small
applet for gnome... (Perhaps this discussion belongs to upstream though. If yes,
please tell the message. :))

Anyways, I'd like to see smart/smartpm in extras-development soon. Don't hold it
here at bugzilla for too long.

Oh, about the package name (smart vs. smartpm)... If you feel strong about
smartpm instead of smart, why not ask the upstream smart developers if they
could change the tar name?

Apparently the text UI isn't really an UI (where you select commands from lists
or something) but a shell (where you write the next command) (launched with
command "smart --shell" when the GUI is started with command "smart --gui"). So
I'd say the package names could be smart-shell and smart-gui for those
subpackages (at least all users would know instantly what the subpackages would
be for).

Whether it is possible to separate those from the main package is another issue
(what if you don't have the gui package installed and you run "smart --gui" - I
hope you won't be seeing a segfault at least). But just my opinion about the naming.

Hope to see smart/smartpm in the repo soon so we can start using it in
production. Thanx.

What makes Gnome so special that you *expect* a 'gsmarttray' package? There is
no Emacs module or Enlightment applet neither...

See the first comments about splitting the text/gtk functionality.

Of course I don't expect the packager to provide the gnome applet. But I hope
upstream will provide it in the future (there is a bug about this in smart bug
tracker IIRC).

That "GUI is not GUI" was a bit weird comment. It is a GUI written using gtk
libraries. I think there are other graphical user interface packages in the
fedora repositories which are named like *-gui and which require gtk. Well,
there are also *-gtk packages. I guess there is no policy about this thing. But
I think the amount of the *-gui packages is a bit higher than the amount of
*-gtk packages.

So what happens if I don't have the graphical user interface package installed
and I run "smart --gui"? Sorry, I would know that if I'd install. But I'm
waiting for the official fedora extras version of smart. ;)

Are you saying rpm has a bug that it allows executing the post scripts in wrong
order when installing multiple packages at once? Even if that's true, I wonder
why are you trying to fix it here with the Requires(pre/postun) stuff. You
should be filing a bug against rpm (or yum/apt-rpm/smart if the bug appears
using them) instead.

I wonder why you need root permissions to just check if there are new updates
available. Well, I know why. Because "smart update" needs root permission to
download the new repo data. I'm just wondering why smart can't just check if the
repo data is even newer on the servers (you can do that with HTTP headers).

from #smart:

<mvo> jval: because it would only know that there is new repo data available,
but not if it affects any of your installed packages
<jval> that was related to the smart applet which just checks if there are
updates available
<jval> but true, even though there would be a newer repo data it would not
necessarily mean new updates for the particular user
<mvo> exactly
<mvo> it could get the update into some sort of private repository, but that's
expensive diskspace-wise
<jval> true

from #smart:

<niemeyer> smart is the executable name, and will continue to be.. Naming the
package after the main executable is usually a good idea, when at all feasible
<niemeyer> So yes, I suggest to use "smart"..

niemeyer = Gustavo Niemeyer (smart lead developer), so we heard the official
word. This package is "smart" and that's final.

I have only one thing I'd like to see changed in the spec file before it is
approved:

I strongly think packages should always state all their dependencies. You should
not assume anything. Smart requires rpm-python (well, at least in fedora it
does), in this case >= 4.3 (or is it 4.4 after all?). That should be in the spec
file.

rpm-build >= 4.3 should be in build requirements because older versions won't
build this package (was it 4.3 or 4.4?).

I suggest adding these for the main package:

Requires: rpm-python >= 4.3
BuildRequires: rpm-build >= 4.3

Other than this, I'd say the spec is ok and ready for release.

One thing though. :)

Does the applet allow running "smart update" by normal users? By _all_ users?
That is not safe (I know it does not do any serious harm, but you should still
not allow it by default - even though it only updates the cache - cache updates
should be controlled by the superuser).

Actually the applet should not run "smart update". There should be a cron job
which does that. The applet should just check if there are new updates - without
trying to update the local cache. If the applet would just do that, it would not
need root permissions. Of course you need root permissions to actually upgrade
the packages, but then you launch the gui which asks password with consolehelper.

Folks, the comments here are getting exceedingly verbose, let's try to keep
things a bit more concise, ok? :)

To summarize, there are multiple items in the latest package that are against
(what I perceive as) the concensus between various reviewers:

- -usermode should be folded into the main package
- add >= 4.3 versions to rpm-python and rpm-build dependencies
- Requires(pre/postun) abuses should be pruned
- -gtk should be renamed to -gui

FWIW, I'm not going to approve the package as long as it directly contradicts
several independent reviewer comments.  On the other hand, I don't insist being
the "assigned to" reviewer here either...

My opinions:


- -usermode should be folded into the main package

* No because the graphical user interface and applet packages only require it
and will bring it in when needed. We don't want new dependencies for the main
package if they aren't really needed.


- add >= 4.3 versions to rpm-python and rpm-build dependencies

* Yes because this package requires those (rpm-python >= 4.3 for running it,
rpm-build >= 4.3 for building it).


- Requires(pre/postun) abuses should be pruned

* Yes because you should not try to fix rpm/yum/apt bugs here. This is not a
blocker for me though because they don't break anything.


- -gtk should be renamed to -gui

* Yes because it tells the user what it is for. You should think like the user
here, not like the developer (User knows that she wants to run "smart --gui".
She doesn't know which toolset/library the --gui uses.).



Any comments about the applet and root permissions issue? I wonder why it is
implemented like that when a cron job for the package database/cache (whatever
it is called) update (I mean "smart update") would simplify things a lot. And
make it much more secure too (you would need root permissions to change anything).

> Does the applet allow running "smart update" by normal users? By
> _all_ users?

Yes, no.

=======


> - -usermode should be folded into the main package

-usermode introduces additional, non-trivial dependencies and is not
needed for core functionality. Therefore, it must be in a separate
package.


> - add >= 4.3 versions to rpm-python and rpm-build dependencies

I guess, this is used to require a certain upstream version. But rpm
lost its ability to require a certain *upstream* version some years
ago when it was changed to non-existing-epoch == Epoch: 0.

Every supported environment (FC-4+) has an rpm version >= 4.3, so the
versioning does not make sense.


> - Requires(pre/postun) abuses should be pruned

When a package fills files into a directory which is not owned by this
package, it MUST be made sure that this directory exists BEFORE the
package is unpackaged. Ditto for uninstalling.

Requires(pre/postun) is the only way which *guarantees* that.


> - -gtk should be renamed to -gui

I heard no technical reasons for that and because I like '-gtk' more,
I am not going to change that.

Hi
> 
> 
> > - -gtk should be renamed to -gui
> 
> I heard no technical reasons for that and because I like '-gtk' more,
> I am not going to change that.

one reason not to name it after the toolkit is that the details about which
toolkit it is written in is not very relevant to the user. If a few tools are
written in gtk and other few are written in qt, fltk and so on and if all of
them are *-gui then it enforces some consistently. Also note that a number of
packages within FC already follow this convention which users are accustomed
towards. 

Re: comment 47: those arguments have already been posted earlier here.  So have
the counterarguments, and because there's no new info, as far as I'm concerned
the change requests are still in effect.

Do you have numbers about the importance of the toolkit? For me, the used
toolkit is very important and I am a user.

> Yes, no.

Ok, so it is secure enough. You need to add the user to a certain group or to
sudoers to allow?

> -usermode introduces additional, non-trivial dependencies and is not
needed for core functionality. Therefore, it must be in a separate
package.

Yes, I strongly agree with you here.

> Every supported environment (FC-4+) has an rpm version >= 4.3, so the
versioning does not make sense.

Ok, but you still should add "Requires: rpm-python".

> I heard no technical reasons for that and because I like '-gtk' more,
I am not going to change that.

The "technical reason" is the "name" of the gui executable "smart --gui" which
maps to "smart-gui" better than to "smart-gtk". Smart does not advertise a "gtk
mode" but a "gui mode".

You don't have to bring the toolkit to the package name even though you think it
is important to the user. It is important for me too as a user, but I don't read
that information from package names but from their requires.

> You need to add the user to a certain group or to sudoers to allow?

I just looked the source. The applet executes "sudo -i $prog" there. So it is up
to the user to allow execution of smart-update in sudoers. Although this might
be a bit confusing for total newbies who don't know anything about sudo and who
have used to using apps which ask for root password, this is actually the best
solution to do this because this method is the most flexible one.

The requirement for rpm-python seems to be there already (sorry if it was there
all a long :)).

I have only one request: change smart-gtk to smart-gui and I'll say let's
approve. :)

There are both names out there in the repo, -gui and -gtk. There seems to be a
"policy" that frontends are usually named as -gui when libraries are named as
-gtk. There are some frontends which are named as -gtk though, but those seem to
have their executable also named like that. All (or at least majority of)
graphical frontend packages seem to be named as -gui.

I'd like to follow that "policy" here. Especially when the gui is launched with
the command "smart --gui". (I know the interface dir is named as "gtk" but I
think it would be more consistent to follow the visible side of it.)

Just wanted to try to convince you more and explain why we others request the
name change.

Other than this, I'd say the spec is ok. Do others still want to bring
smart-usermode to the main package? I really think that it is perfect like it is
now because smart-usermode will be installed with the graphical packages
automatically as dependencies. And we don't want to add unnecessary deps.

Remaining issues/questions refered to the current spec file:

1) ksmarttray doesn't require any kde or qt libraries. Is that ok? It has build
requirement for kdelibs-devel, so I would assume it requires kdelibs perhaps?

2) smart-gtk should be changed to smart-gui because all other binary
distributions of smart use smart-gui as the name and as Rahul Sundaram said: "a
number of packages within FC already follow this convention which users are
accustomed towards" (other reasons above).

3) rpm version numbers should perhaps be added to the requirements anyway
because even though you can't techically trust those, they would tell
users/builders/hackers/whoever important information that rpm version needs to
be at least 4.3.

ad 1) If there is a compiled executable linked to a library in the package, RPM   
automatically picks up the soname dependency, so an explicit "Requires:   
kdelibs" should not be needed (and is in fact discouraged by the Fedora Extras   
packaging guidelines). Explicit library dependencies are only needed for   
interpreted programs (which is why the smart GUI has an explicit dependency on  
pygtk2). 

I wrote this before I saw the above comment from Kevin Kofler:

--

Perhaps you have left out kdelibs from ksmarttray requirements in purpose
because rpm will add the library files as automatic file deps anyway. It seems
the majority of the packages doesn't say they require glibc for example. They
just require libc.so.6 as an automatic file dep.

So this wasn't important really. And if you think about it, I'd say the file
deps are in a sense more accurate even as they only require what is absolutely
necessary and also don't take a stand about which package should provide them
(less to maintain in the spec file). Well, you can't think the auto deps will
always take care of everything but with C-libraries they do just that. And
kdelibs here is a package which contain C-libraries.

--

Good to know there is a policy about this. Kevin Kofler, can you say if there's
also policy for the gui package name issue and the rpm version number issue? A
policy would define what to do here so we would not have to argue about this matter.

So just forget about the first issue, only two left if you ask me (I don't count
the usermode issue because I agree with Enrico Scholz there). :)

1) smart-gtk --> smart-gui
2) rpm version numbers to requirements

Ok, I'll read the spec file line by line and comment here. I'll be commenting
every little detail. Sorry if you don't like my style, but I think it's better
to say everything at once - even minor details.

Ok, here we go...

- Why are the source and patch numbers not growing from 1 to 2 etc. (Not really
an issue. I just wonder if there's some technical reason for that.)

- Packages don't usually use that precise build root dir. Perhaps they should
though and here this is done how it should be done. :) (Not really an issue either.)

- I would not provide smartpm-* there because this package is named smart and
because this will be the initial one there should not be need for those
provides. One name for one software.

- What's smart-plugins? Newer heard. If you made that up as "this would be a
good idea to have" I'd say don't, because there should not be provides which
don't really mean something. I looked quickly at dag's spec file and at least
there is no smart-plugins provided. If you thought this package provides smart
plugins that is not correct as the gui for example is not in this package. Or
perpaps I didn't get it...

- Don't provide smart-tui because the shell is not a tui. There could be a real
tui in the future (dialog style). Shell is not a dialog. I'd say provide
smart-shell because of this - and because "smart --shell" is something the user
knows.

- Require rpm version >= 4.3 even though it can be overridden with epoch. Just
so that the package will tell that rpm >= 4.3 should be used.

- Change smart-gtk to smart-gui. And mention that the frontend is graphical so
that the less techies out there understand what it is.

- Remove obsoletes from smart-gui (if you agree to change the name to
smart-gui). Don't provide smartpm-gui or smartpm-gtk because this is smart, not
smartpm. Things would be different if smart would have already been packaged
somewhere as "smartpm" and there would be possible requirements for it in other
packages.

- If smart-gui (or smart-gtk) belongs to Applications/System, how can ksmarttray
belong to System Environment/Base? A mistake? Should be the same as the gui package?

- Shouldn't smart-usermode obsolete smart-update and not ksmarttray? I might be
wrong but if you just read the decriptions you get that impression.

- The gui package description should say more clearly that it provides a
graphical user interface / frontend. Non-techies don't know what a GTK frontend
means.

- Does %configure contain $RPM_OPT_FLAGS? It would be good to build the package
using optimizations. Also, is there a macro for the make command (I think there
is)? Perhaps use that and not just "make"?

- Same goes for "install". Perhaps use the macro instead?

- You use "touch" there. Shouldn't you require /bin/touch then? Or is there a
macro for it instead?

- You use "test" there. The same thing here. Either there is a macro or you
should require "/usr/bin/test".

- You use "gtk-update-icon-cache". Shouldn't you require
"/usr/bin/gtk-update-icon-cache" then?

- And require those commands in the correct package (if used when installing a
subpackage, require in subpackage).

Ok, done. You have plenty of issues to comment now. Perhaps I was nitpicking but
at least I was reading the spec precise enough so I don't have to reread it and
write more again. :)

(In reply to comment #35)

> GUI is not GUI (gtk apps do not work fine in non-gnome environments),
> so I like it when the used toolkit is visible in the package name.

Um, that isn't strictly true.  You can run GNOME/GTK apps in a KDE environment 
(e.g. abiword runs fine in KDE) and vice-versa, so long as the base libraries
are installed, you don't actually need to be running that particular.  Also KDE
apps (such as scribus) work fine in GNOME.

I've run smart in KDE (or even FVWM2 for that matter) and it works just fine.

So I vote for the subpackage to be named smart-gui not smart-gtk.

That's a long list in comment #58 compared to the just-three-items-left list a
couple of days ago. :(

We don't need to make smart *the perfect package*, it's more important to get
its review finalized, before Enrico gets pissed off too much.

But I'd like to comment on the suggestion to use macros for make/test etc. For
one using a macro there would not replace a Requires:. Either the package in
question is in the default Requires: list, or it needs to be set as a dependency
independently of whether you use macros to obfuscate the specfile.

And I already mentioned the second part about macros: If all they do is replace
make with %{__make} then all they do is obfuscate the specfile beyond
recognition. Please don't do that! You may argue that %{__foo} will have an
absolute path and is therefore safer etc., but the build itself has to be
assumed to be in a safe environment and for packages submitted here even in a
minimal chroot w/o any broken remnants under /usr/local.

> Good to know there is a policy about this. Kevin Kofler, can you say if   
> there's also policy for the gui package name issue and the rpm version   
> number issue? A policy would define what to do here so we would not have   
> to argue about this matter.   
   
I don't think there is, otherwise it would have been brought up already by the   
people knowing more than me. ;-)   
   
> - Shouldn't smart-usermode obsolete smart-update and not ksmarttray? I  
> might be wrong but if you just read the decriptions you get that impression.  
  
Other ksmarttray packages are linked against smart-update, so doing what you  
suggest could cause conflicts. Obsoleting it in ksmarttray doesn't have that  
problem.  
  
> - Same goes for "install". Perhaps use the macro instead? 
> 
> - You use "touch" there. Shouldn't you require /bin/touch then? Or is there a 
> macro for it instead? 
> 
> - You use "test" there. The same thing here. Either there is a macro or you 
> should require "/usr/bin/test". 
 
I think these are base tools which can be assumed to just be there. 
 
> - You use "gtk-update-icon-cache". Shouldn't you require 
> "/usr/bin/gtk-update-icon-cache" then? 
 
That makes sense (if it isn't already being required - it's part of the gtk2 
package), it should be a Requires(post) though. 

>> - You use "gtk-update-icon-cache". Shouldn't you require 
>> "/usr/bin/gtk-update-icon-cache" then? 
 
> That makes sense (if it isn't already being required - it's part of the gtk2 
> package), it should be a Requires(post) though. 

Wrong.  From "ScriptletSnippets" wiki page regarding "GTK+ icon cache":
"Note that no dependencies should be added for this"

Oops, so this is a moot point too. 

Sorry, there's more noise and opinions here than what I'm willing to spend time
working with at the moment -> back to FE-NEW for someone else to take over the
review responsibility.

>> - Shouldn't smart-usermode obsolete smart-update and not ksmarttray? I  
>> might be wrong but if you just read the decriptions you get that impression.  
  
> Other ksmarttray packages are linked against smart-update, so doing what you  
> suggest could cause conflicts. Obsoleting it in ksmarttray doesn't have that  
> problem.

Are linked against? Do you mean that other ksmarttray packages require
smart-update? In that case I don't understand your reasoning because I think
smart-usermode replaces smart-update in this spec. (And doesn't replacing mean
obsoleting?)

If you mean other ksmarttray packages provide smart-update, you forget that they
also provide ksmarttray. Obsoleting smart-update in smart-usermode doesn't mean
ksmarttray would be removed if the user has ksmarttray installed. Our ksmarttay
will replace the old ksmarttray.

The only situation I can think of where this would actually cause conflicts
would be a one where the user tries to take ksmarttray and
smart-update/smart-usermode from different repositories. And even then the
conflict would happen only if the user would try to take the same version of
those packages from different repositories.

Even if such a conflict would happen you forget that fedora-extras doesn't (and
shouldn't) have to support third party repositories like that. The third parties
should remove smart from their repos when fedora-extras starts providing it.

* Sat Jan 28 2006 Enrico Scholz <enrico.scholz.de> - 0.41-0.6
- renamed '-gtk' subpackage to '-gui-gtk' to please everybody
- changed group of 'ksmarttray' to Applications/System
- added 'Requires(post): gtk-update-icon-cache' for the -gui-gtk
  subpackage, but NOT for %postun or the ksmarttray package. It does
  not matter whether gtk2 exists at uninstallation: when it exists,
  the cache will be updated; else, the cache is outdated but there is
  nothing which can use it.
  To avoid 'gtk2' dependencies in the KDE subpackage, use a %triggerin
  there for updating the icon-cache and ignore missing programs in
  %postun silently for the reasons told above already.
- added a 'Requires(post): bash' for the main package because a bash
  script is invoked there.
- updated empty-description patch to use this one from upstream SVN
  (rev690+691)


http://ensc.de/fedora/smart.spec
http://ensc.de/fedora/smart-0.41-0.6.src.rpm


==============



> - Why are the source and patch numbers not growing from 1 to 2 etc. (Not
>   really an issue. I just wonder if there's some technical reason for
>   that.)

Allows grouping of related patches/sources without the need to renumber
everything when a new patch/source is added.


> - What's smart-plugins?

The problem with plugins is, that they usually add new dependencies
which are not need for the core functionality. So my first 'smart'
package had a physical '-plugins' subpackage. Because *current*
plugins do not add mentioned dependencies, I merged it back into the
main package.


> - Don't provide smart-tui because the shell is not a tui. 

'smart --shell' is a textual interface for user interaction. So, it
can be labeled 'TUI'... But ok, to make it clear, I renamed it to
'-tui-shell'....


> - Require rpm version >= 4.3 even though it can be overridden with
>   epoch. Just so that the package will tell that rpm >= 4.3 should
>   be used.

README should tell this also... Again: there is absolutely no technical
reason to add versioned dependencies here; the supported environment
(FC4+) has all the needed upstream versions.


> - Change smart-gtk to smart-gui.

Ok, you have won. I renamed it to 'smart-gui-gtk' which should please
everybody.


> - If smart-gui (or smart-gtk) belongs to Applications/System, how
>   can ksmarttray belong to System Environment/Base?

Thx, fixed.


> - Shouldn't smart-usermode obsolete smart-update and not ksmarttray?

'smart-usermode' and 'smart-update' are having completely different
purposes. 'smart-usermode' uses the PAM based userhelper which is
intended for sporadic execution by the user.

'smart-update' is called regularly by 'ksmarttray'; upstream and some
other repos ship it as a SUID wrapper. I think, this is a horrible idea
because of security reason and made it a sudo wrapper. Because only
'ksmarttray' uses it, this package has the needed Provides/Obsoletes.


> - The gui package description should say more clearly that it
>   provides a graphical user interface / frontend.

ok, added a 'GUI' there...


> - Does %configure contain $RPM_OPT_FLAGS?

yes


> - Same goes for "install". Perhaps use the macro instead?

which macro?


> - You use "touch" there. Shouldn't you require /bin/touch then? Or
>   is there a macro for it instead?

'rpmbuild ...' environments should have coreutils installed


> - You use "test" there. The same thing here. Either there is a macro
>   or you should require "/usr/bin/test".

'test' is a bash builtin


> - You use "gtk-update-icon-cache". Shouldn't you require
>   "/usr/bin/gtk-update-icon-cache" then?

thx, it's in the Requires(post): of the -gui-gtk subpackage now; it
does not make sense at other places.


> - And require those commands in the correct package (if used when
>   installing a subpackage, require in subpackage).

???



=======


> Um, that isn't strictly true.  You can run GNOME/GTK apps in a KDE
> environment (e.g. abiword runs fine in KDE) and vice-versa

no; Gnome2 apps do not work fine in non Gnome2 environments because
important configuration settings (e.g. larger fonts) will not be
applied.

(In reply to comment #66)

> > Um, that isn't strictly true.  You can run GNOME/GTK apps in a KDE
> > environment (e.g. abiword runs fine in KDE) and vice-versa
> 
> no; Gnome2 apps do not work fine in non Gnome2 environments because
> important configuration settings (e.g. larger fonts) will not be
> applied.

They still work "fine" for the average user modulo font/theme issues, especially
if they stick with the default Fedora Core themes (i.e. Bluecurve and Clearlooks
too, I think) which has been customised to look more or less the same whether
running GNOME or KDE apps.

No, drop Requires(post): gtk-update-icon-cache
I repeat: From "ScriptletSnippets" wiki page regarding "GTK+ icon cache":
"Note that no dependencies should be added for this"

> > no; Gnome2 apps do not work fine in non Gnome2 environments because
> > important configuration settings (e.g. larger fonts) will not be
> > applied.
>
> They still work "fine" for the average user modulo font/theme
> issues, especially if they stick with the default Fedora Core
> themes (i.e. Bluecurve and Clearlooks too, I think) which has been
> customised to look more or less the same whether running GNOME or
> KDE apps.

I do not care about the "average user" (whoever this is). I care about
the users whose environment I have to administrate. There, large fonts
are *required*.


> No, drop Requires(post): gtk-update-icon-cache

I do not see, how the icon-cache will be updated in the

1. smart
2. gtk2

installation sequence (which would be possible without the Requires(post)).

AFAIS, FC4's gtk2 does not have a script/trigger to update the
icon-cache. So, all gtk2 applications will have a 5s startup
penalty with the sequence above.

(In reply to comment #69)

> > They still work "fine" for the average user modulo font/theme
> > issues, especially if they stick with the default Fedora Core
> > themes (i.e. Bluecurve and Clearlooks too, I think) which has been
> > customised to look more or less the same whether running GNOME or
> > KDE apps.
> 
> I do not care about the "average user" (whoever this is). I care about
> the users whose environment I have to administrate. There, large fonts
> are *required*.

Well abiword runs just fine under KDE, and we don't call it abiword-gtk and it
has the same font problems as smart-gui.  Anyway, I think that I can live with
smart-gui-gtk, so long as we don't *prevent* it from working with KDE... ;-) 
(and it should still be visible in the KDE menus as many of the Fedora/Red
Hat-specific configuration tools which are GTK/GNOME based are currently visible).

Although I would still prefer smart-gui, I'm not doing the review, so I think we
can drop this discussion.

> no; Gnome2 apps do not work fine in non Gnome2 environments because 
> important configuration settings (e.g. larger fonts) will not be 
> applied. 
 
Write your font/theme settings into the system-wide /etc/gtk-2.0/gtkrc (by 
hand, as none of the config tools write there). That way, the settings will get 
applied at system or application startup, without needing some GNOME theme 
manager to run. (Even rhgb gets the settings.) That's what I'm doing to force 
my GTK+ apps to use Bluecurve instead of Clearlooks (I want them to match my 
KDE desktop, not to look "pretty" or "modern", Bluecurve has worked well since 
it was introduced in RHL8 and I don't see the point of yet another theme - but 
that's a different flamewar ;-) ) and it works perfectly. 

> The problem with plugins is, that they usually add new dependencies
> which are not need for the core functionality. So my first 'smart'
> package had a physical '-plugins' subpackage. Because *current*
> plugins do not add mentioned dependencies, I merged it back into the
> main package.

I bet you will never ship smart-plugins because when a plugin adds that kind of
dependencies you will make a separate subpackage for that specific plugin, not
for all plugins. This is the reason I'd say don't provide this smart-plugins
package.


> 'smart --shell' is a textual interface for user interaction. So, it
> can be labeled 'TUI'... But ok, to make it clear, I renamed it to
> '-tui-shell'....

I'd still follow the usage "smart --shell" and name this as "smart-shell". But,
as you don't ship a separate smart-tui, smart-shell or smart-tui-shell
subpackage, I'd say don't provide it. The reason is to not pollute the package
namespace with packages which doesn't really exist and which have never even
existed. (For the same reason I'd say don't provide smartpm-* packages.)


> README should tell this also... Again: there is absolutely no technical
> reason to add versioned dependencies here

Is there a policy for this? There might be because this is a very general issue;
whether to issue all version requirements or only those which might not be
satisfied already in the distribution.


> Ok, you have won. I renamed it to 'smart-gui-gtk' which should please
> everybody.

I'd still like to follow the usage "smart --gui" here and name the package as
"smart-gui". I don't like long names... And I don't like names which don't
follow the upstream names... And I don't like names which try to tell things
like the used library etc... Those namings belong to Debian policy (which I
think causes more confusion than help - again: rpm --requires tells the toolkit
etc.). Fedora policy is to name packages like they are named in upstream.


> 'rpmbuild ...' environments should have coreutils installed

I think you are right there. Only those requirements should be issued which
aren't already provided by other requirements (although you should be 100% sure
they really are provided). And in this case /bin/touch is provided by coreutils
which provides fileutils which in turn is required by rpm. :)


> which macro?

%{__install} (from /usr/lib/rpm/macros). In general, I'd use the most common
macros - at least those from /usr/lib/rpm/macros because those should not add
any extra dependencies.


> 'test' is a bash builtin

Yes, sorry. I really noticed "which" is not very usefull command - "type" is
much better. :)


> ???

Forget. I was just talking in general there.

Anyone else willing to take up the review now that Ville has resigned (comment
#64)? (I'm not an official contributor yet, so I can't).

smart-update requires sudo >= 1.6.8 due to use of "sudo -i".  Can't the -i be
just dropped?  That would make things work with RHEL4.

ksmarttray invokes "smart --gui" for installing the available updates; even
though that would bring in the gtk dependencies to kde systems, I think a
dependency on smart-gui should be added, perhaps using Requires(missingok).

(In reply to comment #69)
> 1. smart
> 2. gtk2
> installation sequence (which would be possible without the Requires(post)).

Please demonstrate how that sequence would be possible without it.  I'm assuming
you mean smart-gui-gtk, not smart in the sequence though.  See also
https://www.redhat.com/archives/fedora-extras-list/2006-February/msg00322.html

> smart-update requires sudo >= 1.6.8 due to use of "sudo -i".  Can't
> the -i be just dropped?  That would make things work with RHEL4.

Does this sudo support the '-H' flag?


>> 1. smart
>> 2. gtk2
>> installation sequence (which would be possible without the Requires(post)).
>
>Please demonstrate how that sequence would be possible without it.

E.g. a new package like

| Name:		ncgl
| Summary:	A new, cool, libGL.so implementation
| # We need smart to download the firmware, and users want a GUI
| Requires:	smart-gui-gtk


will create a circular dep like

   xorg-x11-libs -> libGL.so  -> smart-gui-gtk -> gtk2
       ^           (ncgl wins)                     |
       `-------------------------------------------'


It is not specified whether smart-gui-gtk or gtk2 will be installed
first.

Such an 'ncgl' package does not exist in Fedora Extras currently, but:

* could be added in the future; I do not have the time to investigate
  all possible circular deps when a new package gets added

* could be added by a third party repository. E.g. a well known one
  provides 'ati-fglrx' which wins against the default
  xorg-x11-Mesa-libGL


That's why, I prefer to prevent such undetermined situations with
tagged Requires(...).

Yes, sudo 1.6.7p5 understands -H.

The circular dep example is too far fetched for me to consider it relevant, and
I guess it's just as easy to craft artificial situations where use of the
Requires(post) would cause problems as it was to come up with that example. 
(No, I'm not willing to spend time trying to demonstrate one though.)

FYI, since progress seems to have stalled here, an alternative and partially
superseding package is up for review in bug 185239 in case folks feel more
comfortable with that.

In that case I'll go ahead and reopen bug 175630 and bug 175631, too.
Is there some guideline as how to proceed in such cases?

Looks like bug #175630 has resulted in an approval, smart is now built and
available.  Shouldn't this bug be closed as either WONTFIX or DUPLICATE?

*** This bug has been marked as a duplicate of 175630 ***