Bug 548522

Why all the hassle with git patches? What's wrong with find and chmod?

Are the patches sent upstream?

(In reply to comment #1)
> Why all the hassle with git patches? What's wrong with find and chmod?

The initial packaging work was done by Jesse Keating.  Git is being used during the build as it is easier for the packager to manage keeping the local patch set in sync and in a format acceptable for submission upstream.

> Are the patches sent upstream?  

Patch1: 0001-Change-usr-local-to-usr-share-where-necessary.patch

This patch is specific to where Fedora prefers package content to live.  Upstream currently uses /usr/local/autotest, and it was determined that /usr/share/autotest was a more appropriate place for Fedora.  Understandably, there isn't interest in taking this patch upstream.

Patch2: 0002-Set-file-permissions-appropriately.patch
Patch3: 0003-Don-t-hardcode-python2.4.patch

These patches are already fixed upstream, but not in the current 0.11.0 release of autotest.  They will however be included in upstream 0.11.1 and future releases.

Koji scratch-build of autotest-client available at http://koji.fedoraproject.org/koji/taskinfo?taskID=1907647

The remarks about the patches you gave in comment:2 should go in the spec as comments near the PatchX lines.

Why do you set the default owner/group to autotest? It seems to be wrong, e.g. /usr/share/doc/autotest-client-0.11.0/LICENSE does certainly not need to be owned by autotest:autotest.

Also the directory structure does not seem clean, e.g. I guess /usr/share/autotest/client/bin/autotest should go to /usr/bin/autotest.

(In reply to comment #4)
> The remarks about the patches you gave in comment:2 should go in the spec as
> comments near the PatchX lines.

Okay, as directed, I've added my comments to the autotest-client.spec file.

See updated autotest-client.spec - http://jlaska.fedorapeople.org/autoqa/autotest-client.spec

> Why do you set the default owner/group to autotest? It seems to be wrong, e.g.
> /usr/share/doc/autotest-client-0.11.0/LICENSE does certainly not need to be
> owned by autotest:autotest.
> 
> Also the directory structure does not seem clean, e.g. I guess
> /usr/share/autotest/client/bin/autotest should go to /usr/bin/autotest.    

Unfortunately, it cannot be fully packaged as you might expect new software to be packaged in Fedora.  Autotest is packaged upstream such that *everything* lives in it's own root-directory (much like other web-services like mediawiki or moin).  The source-code for autotest expects this.  We investigated carrying patches and moving content into a directory structure that was more familiar to most people, however that patchset would be *enormous*.

I can change the default owner back to root:root, and identify the exactly sub-directories that autotest requires write access to if you prefer.  But it may not be possible to have everything root:root under /usr/share/autotest for autotest-client.

(In reply to comment #5)
> (In reply to comment #4)

> > Why do you set the default owner/group to autotest? It seems to be wrong, e.g.
> > /usr/share/doc/autotest-client-0.11.0/LICENSE does certainly not need to be
> > owned by autotest:autotest.
> > 
> > Also the directory structure does not seem clean, e.g. I guess
> > /usr/share/autotest/client/bin/autotest should go to /usr/bin/autotest.    
> 
> Unfortunately, it cannot be fully packaged as you might expect new software to
> be packaged in Fedora.  Autotest is packaged upstream such that *everything*
> lives in it's own root-directory (much like other web-services like mediawiki
> or moin).  The source-code for autotest expects this.  We investigated carrying
> patches and moving content into a directory structure that was more familiar to
> most people, however that patchset would be *enormous*.

Is it really a webservice? It does not look like one. Imho for proper packaging, at least some changes to the structure need to be made like moving the binaries that are supposed to be called by a user /usr/bin and maybe some stuff from tools. Or if these scripts are only supposed to be run a the service, then they should be moved to /usr/libexec.


> I can change the default owner back to root:root, and identify the exactly
> sub-directories that autotest requires write access to if you prefer.  But it
> may not be possible to have everything root:root under /usr/share/autotest for
> autotest-client.    

Yes, please do this. But imho the directories that require write access should also be moved below /var. But I somehow doubt, that would be enough. Why is the autotest user even necesary? There is no init script that seems to run the daemon with this priviliges. Is a user sopposed to su to autotest to use the package?

(In reply to comment #6)
> Is it really a webservice? It does not look like one. 

You are correct, the autotest service (which will be put up for review later) contains the front-end web service portion.  

> Imho for proper
> packaging, at least some changes to the structure need to be made like moving
> the binaries that are supposed to be called by a user /usr/bin and maybe some
> stuff from tools. Or if these scripts are only supposed to be run a the
> service, then they should be moved to /usr/libexec.

> > I can change the default owner back to root:root, and identify the exactly
> > sub-directories that autotest requires write access to if you prefer.  But it
> > may not be possible to have everything root:root under /usr/share/autotest for
> > autotest-client.    
> 
> Yes, please do this. But imho the directories that require write access should
> also be moved below /var. But I somehow doubt, that would be enough. Why is the
> autotest user even necesary? There is no init script that seems to run the
> daemon with this priviliges. Is a user sopposed to su to autotest to use the
> package?    

Upstream has no intention of refactoring the code layout so that it lines up with a downstream distribution filesystem standards.  While it certainly would be advantageous for Fedora, and personally more familiar and easier to work with, it's not something they are willing to adopt at this time.  I'll be happy to note that in the .spec file as well.

Additionally, I'm not seeing that the strict filesystem guideline adherence is a blocker to package review.  As far as I can tell, the packaging guidelines list that as a *should* (not a *must*) requirement [1].

I will update the autotest-client.spec and rebuild with a more conservative approach to the usr/grp ownership of the /usr/share/autotest directory structure.  This will likely take a day or more, I will post a comment here when updates are available.

Many thanks!

[1] https://fedoraproject.org/wiki/Packaging/Guidelines#Filesystem_Layout

James, at least the "/usr as read-only" principle is one it's really worth observing; some people actually do mount /usr over a network as read-only, and will complain vociferously if something expects to be able to write to it. I'm sort of with Till here, really; autotest does seem to be wilfully doing rather odd things, and I think it'd be perfectly reasonable to patch those even if upstream weren't immediately interested in the patch. It's not a question of 'a downstream distribution's' filesystem standards, this is FHS stuff, which all distros pay at least lip service to.

You didn't seem to answer Till's question as to why an autotest user account is necessary.

The comment "This package is specific to Fedora..." is probably supposed to read "This patch is specific to Fedora..."

The description is quite thin. It might be useful to explain briefly what the hell autotest is.



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

(In reply to comment #8)
> James, at least the "/usr as read-only" principle is one it's really worth
> observing; some people actually do mount /usr over a network as read-only, and
> will complain vociferously if something expects to be able to write to it. I'm
> sort of with Till here, really; autotest does seem to be wilfully doing rather
> odd things, and I think it'd be perfectly reasonable to patch those even if
> upstream weren't immediately interested in the patch. It's not a question of 'a
> downstream distribution's' filesystem standards, this is FHS stuff, which all
> distros pay at least lip service to.

Yeah, it certainly would be cleaner to have it more in line with what most Fedora packages look like.  Unfortunately, that's a pretty aggressive and *unmaintainable* patch.  Also, while it's a *SHOULD* have, it's not a *MUST* have in the packaging guidelines.

    "Fedora follows the Filesystem Hierarchy Standard with regards to
    filesystem layout. The FHS defines where files should be placed on the
    system.  Fedora packages must follow the FHS. Any deviation from the FHS
    should be rationalized when the package is reviewed. " 

    - https://fedoraproject.org/wiki/Packaging/Guidelines#Filesystem_Layout

That said, in my previous comment I noted that I am attempting to address some of the concerns around permssions.  I've not yet completed that work, and will respond with updated packages at that time.

> You didn't seem to answer Till's question as to why an autotest user account is
> necessary.

It's a test harness that includes a local test scheduler.  I believe upstream may run the harness as the root user.  I think that could be improved, so I've added an 'autotest' user/group account to this package.  This comes into play when running the harness on the local system for processing local jobs.  It also is important when running in a distributed setup (provided by the future autotest package).

> The comment "This package is specific to Fedora..." is probably supposed to
> read "This patch is specific to Fedora..."

Thanks, I'll addressed in the latest .spec file.

> The description is quite thin. It might be useful to explain briefly what the
> hell autotest is.

I thought it did that :)  But, I've expanded on it in the latest .spec file.

I note that the spec and srpm links all seem to be 404.  And from reading the commentary, I have a hard time imagining that any package reviewer worth their salt would be willing to approve the FHS violations mentioned.  And to avoid the kind of "upstream does it, and even if it's completely broken I don't want to deviate from upstream" rationalization seen above, I'll propose a guideline change.

For history, the current AutoQA packages (including autotest) can be at http://repos.fedorapeople.org/repos/fedora-qa/autoqa/

A dependency of the autotest package (gwt) is not yet available for review.  I'm not prepared to move forward with 'autotest' until 'gwt' is in a state worthy of package review.  At that time, we can revisit the concerns raised in this review request.  

If automated Fedora QA is desired, assistance will be needed to help massage the current 'autotest' package structure into something more Fedora-friendly.  Upstream has structured their autotest "packaging" in an effort to make it a distributable and usable in any operating system / environment.  That intentional design decision makes it difficult to easily move content out into system-wide locations.  

Nothing is impossible of course, but re-packaging 'autotest' is not a priority focus for Fedora test automation efforts at this time.  I definitely need and welcome expertise+guidance on cleaning up the 'autotest' package in a manner consistent with Fedora packaging guidelines, and sustainable for the Fedora package maintainer.

I'm closing this request for now.  Autotest will likely come back for review soon when we are able to resolve the FHS issues.  When it does, we can either reopen this request, or start a new review.

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