webapp.eclass mishandles SLOT: * It defaults SLOT="${PVR}", which includes the revision part of a package. This means that any vital fixes will be left unapplied even when the user upgrades. * It includes a mechanism for allowing packages to override this, but when said mechanism is used it spits out a nasty warning message telling the user that it's probably a screwup in a package.
Before we start, can we _please_ _please_ have a civilized conversation? I know there's some bad blood between a few people who will or should comment on this bug, and I'd really like to have a productive discussion. Substantitative comments to follow.
> * It includes a mechanism for allowing packages to override this, but when said > mechanism is used it spits out a nasty warning message telling the user that > it's probably a screwup in a package. what about my proposal here: http://article.gmane.org/gmane.linux.gentoo.devel/36386 considering the nature of the warning, it should be trivial to make it into a proper QA check by having the class see where files were installed and then warn/abort if certain conditions are met
Hi, This is a general comment to anyone who wants to join this debate. I think it would be helpful if you were familiar with the design goals of webapp-config. GLEP 11 includes the initial goals, [1] includes a more mature understanding of the problems, and the webapp-config manual page also has a bit more information. In particular, please make sure you're comfortable with how the two-phase install works. This will give us a common frame of reference to work together. > * It defaults SLOT="${PVR}", which includes the revision part of a package. > This means that any vital fixes will be left unapplied even when the user > upgrades. I believe there's a misunderstanding here on how webapp-config works. The webapp eclass runs in two different modes, governed by the 'vhosts' USE flag. 1) USE=-vhosts: The package is installed into /usr/share/webapps/${PN}/${PVR}. The eclass then calls webapp-config to {install|update} the copy under /var/www/localhost/htdocs/${PN}. With USE=-vhosts, I believe that all vital fixes are applied automatically when the user upgrades. If there's a corner case that I've missed in this scenario, please let me know so that we can address it. 2) USE=vhosts: The package is installed into /usr/share/webapps/${PN}/${PVR}. The eclass does not call webapp-config to {install|update} any copies installed under /var/www, /srv, or anywhere else the user has chosen to place their websites. It's the responsibility of the box's sysadmin to separately upgrade each and every install, by running webapp-config manually. There may be hundreds of hosted websites on the box, which is one of the use cases that webapp-config is designed to accomodate. In this case, it is not safe to assume that the sysadmin has the time, or authority, to upgrade all the installed instances of the app in one go. SLOT=${PVR} makes it possible for sysadmins and management to do the upgrades to suit their local policies and politics. To help a sysadmin see how many sites he has left to upgrade, webapp-config includes support for listing all remaining installs of any given version of any given web-app. Switching to SLOT=${PV} currently breaks one of our use cases. Although the default operation is to hard link from /usr/share/webapps to /var/www (if both are on the same filesystem), or to copy from /usr/share/webapps to /var/www (if both are on different filesystems), users can instead choose to symlink from /usr/share/webapps to /var/www. This used to be the default operation, until webapp-config v1.10 was released on 11th June 2004. With SLOT=${PV} and USE=vhosts, emerge -u will break all of those symlinks. I'm personally comfortable with SLOT=${PVR}. It supports a wider range of USE cases than SLOT=${PV} can. However, it's not optimal for the common USE cases, where users do not select symlinking. If we agree that we should move to SLOT=${PV} instead, then it cannot happen straight away. We'll need to follow an action plan similiar to this: a) Educate users that symlink support will be removed from webapp-config, and that they need to migrate their servers over to a non-symlinking strategy. b) Release and stabilise a version of webapp-config with symlink support removed. c) Allow time for users to migrate to the new version of webapp-config. d) Switch to SLOT=${PV} in the eclass. I believe that allowing three months each for (a) and (c) would be reasonable, and consistent with how we try to do things on the web front. That would mean (d) happening sometime in September. I believe that the symlinking use case is a very uncommon one. I doubt we'll upset many users by dropping it. > * It includes a mechanism for allowing packages to override this, but when said > mechanism is used it spits out a nasty warning message telling the user that > it's probably a screwup in a package. The way that the installation of web-based packages work is a major paradigm shift from what developers were used to at the time. The eclass originally didn't have that warning. I was forced to add this because, as a matter of habit, developers were (and still are) used to automatically putting SLOT=0 in their ebuild, which breaks the use cases described above. We've provided a mechanism for ebuilds to suppress this warning if they have a genuine reason to not use SLOT=${PVR}. However, if an ebuild is having to do this, then the chances are that the ebuild isn't really suitable for installation using the webapp eclass in the first place as it currently stands. If users see this warning, it's because a developer has shipped a broken package, and it's the package that needs work, not the eclass. I believe that the warning is a valid QA check. [1] http://www.phparch.com/issuedata/articles/article_77.pdf Best regards, Stu
> We've provided a mechanism for ebuilds to suppress this warning if they have a > genuine reason to not use SLOT=${PVR}. i think your mechanism is broken: if [ "${SLOT}+" != "${PVR}+" ]; then if [ "${WEBAPP_MANUAL_SLOT}" != "yes" ]; then die else warn fi fi how exactly does that allow devs to supress the warning ? if they change SLOT, then the eclass either kills the emerge or it warns, no other option is provided
Sorry, you're right. That code needs changing so that the warning message doesn't appear. Best regards, Stu
> I was forced to add this because, as a matter of > habit, developers were (and still are) used to automatically putting SLOT=0 in > their ebuild, which breaks the use cases described above. Hm. So if you must check for a screwy slot, what's wrong with checking for "0" and allowing anything else?
(In reply to comment #6) > Hm. So if you must check for a screwy slot, what's wrong with checking for "0" > and allowing anything else? My thinking at the time was that I didn't want any ebuilds overriding SLOT at all. The most reliable way I could find was to test that the SLOT set by the webapp eclass hadn't been changed by the ebuild. Anything else seemed like an incomplete check. Best regards, Stu
So, what has the webapp team decided to do concerning this? Are you going to change to using SLOT=${PV}? Personally, I'm not opposed to how it works currently. The issue that Spanky brought up is still present though.
My preference is to stay with SLOT=${PVR}, and to fix the issue that Spanky pointed out. Best regards, Stu
The eclass doesn't display a warning anymore if WEBAPP_MANUAL_SLOT is set. What is QA's opinion now that Halcy0n is retired?
Nothing from QA since July. Our preference is to keep it as PVR.
