jdeb icon indicating copy to clipboard operation
jdeb copied to clipboard

Add support of man-page producers.

Open roman-kashitsyn opened this issue 12 years ago • 15 comments

Lintian wants a man page entry for every executable included into the package and it wants them gzipped. It's inconvenient to add another maven plugin to just gzip a man page entry during build. In addition, it's not so easy to find a correct location for a man page using only the Debian Policy Manual.

This patch provides a new man-page producer type that can guess correct location for a page and gzip it. Unfortunately, it looks impossible to gzip a page with maximal compression level as lintian wants.

In addition, it removes some code duplication by introducing the Producers package-private class to hold common constants and methods and the ProducerFactory class to create producers from run-time configuration in the ant task and the maven plugin.

roman-kashitsyn avatar Dec 11 '13 13:12 roman-kashitsyn

Any comments on this patch?

roman-kashitsyn avatar Dec 13 '13 09:12 roman-kashitsyn

Hi Roman, thank you for the patch. It's quite big and I haven't everything yet, it would have been preferable to split the refactoring from the feature addition.

You can get the best compression level with gzip by subclassing GZIPOutputStream:

GZIPOutputStream out = new GZIPOutputStream(out) {
    {
        def.setLevel(Deflater.BEST_COMPRESSION);
    }
};

ebourg avatar Dec 13 '13 09:12 ebourg

Hello Emmanuel,

Thank you very much for your feedback. I will try to reduce patch sizes in future since I don't like large patches too. Special thanks for your advise with GZIP stream.

roman-kashitsyn avatar Dec 13 '13 10:12 roman-kashitsyn

Unfortunately, the trick with anonymous GZIPOutputStream subclass doesn't work: the stream writes header in a constructor with a hard-coded compression method, it doesn't ask the deflater (if it even were, it would not see the changes made by the anonymous subclass later). lintian is still complain on such a compressed file :(

roman-kashitsyn avatar Dec 13 '13 10:12 roman-kashitsyn

Interesting, that looks like a bug in the JDK. We should be able to work around this issue by modifying the gzip compressor in Commons Compress to write a custom header.

ebourg avatar Dec 13 '13 12:12 ebourg

As I can see, the GzipOutputStream from commons.compress just delegates it's work to java.util.zip.GZIPOutputStream. Possibly, the only option is to write a simple utility using the Deflater directly.

roman-kashitsyn avatar Dec 13 '13 15:12 roman-kashitsyn

Yes, that's what I'm doing currently :)

ebourg avatar Dec 13 '13 15:12 ebourg

Actually, there is a simpler way. The compression level is encoded in the header flag XFL (http://www.gzip.org/zlib/rfc-gzip.html) that is now hard-coded to zero. Since we have the compressed file loaded to memory, we could use the trick with anonymous subclass and patch the XFL flag to be 2. I'll try to implement that.

roman-kashitsyn avatar Dec 14 '13 04:12 roman-kashitsyn

Commons Compress now has a better implementation of the gzip output stream. Could you give it a try Roman? The code is on the trunk and will be part of the 1.7 release:

http://svn.apache.org/repos/asf/commons/proper/compress/trunk/

ebourg avatar Dec 16 '13 13:12 ebourg

Yep, it's what is needed. I suppose, the pull request merge is delayed until 1.7 release of commons-compress since it's not a good idea to copy the stream implementation into the jdeb source tree.

roman-kashitsyn avatar Dec 16 '13 17:12 roman-kashitsyn

Commons Compress 1.7 has been released if you want to rework your patch Roman.

Regarding the additions of a producer, I'm not a big fan of this solution. Following this trend we may quickly end up with a myriad of producers addressing various types of files. jdeb could be smarter and transform automatically the input files to comply with the Debian Policy (compress automatically the man pages, make the files in /usr/bin executable,etc). This could be performed by a filter mechanism, I haven't figured how to implement it though.

From a user point of view it would be easier to just say "package this set of files" and let jdeb do the right things than having to learn the subtleties of the Debian Policy and handle every special case with a specific producer.

ebourg avatar Jan 28 '14 11:01 ebourg

I think the alternative for the producer is another <data> option

<man-page category="N" prefix="/usr/share/man/manN" compressor="gzip"/>

with reasonable defaults for all the attributes.

roman-kashitsyn avatar Jan 28 '14 12:01 roman-kashitsyn

I hate to see a contribution wasted but I think this probably should be solved differently. We could accept it for 1.x (not eager) ...but then we should really start thinking of 2.x to solve this properly.

tcurdt avatar Feb 13 '14 01:02 tcurdt

I was busy with other stuff, but I'll definitely return to this feature in short time. I appreciate any ideas how to implement the documentation support in more general and non-intrusive way. It's also unfair to provide man pages but ignore info pages. I'll rework the solution as soon as I find a good design for it. Sometimes no feature is better than poorly implemented one...

roman-kashitsyn avatar Feb 13 '14 06:02 roman-kashitsyn

Actually, the main problem here is compression. The problem with hard-to-find man page location could be easily solved with a couple of examples in the jdeb manual. So, I think it's enough to provide ability to compress files on archive construction. I think we don't need any new producers here, just an additional data attribute (e.g. compression):

<data>
  <src>${project.basedir}/src/main/resources/app.1</src>
  <type>file</type>
  <compression type="gzip"/>
  <mapper>
    <type>perm</type>
    <prefix>/usr/share/man/man1</prefix>
  </mapper>
</data>

This also should work for info pages.

The other solution here is to (optionally) scan maven resources and locate all the man pages and info pages, compress them and put into the package. This solution fits maven philosophy better but doesn't look composable with other parts of jdeb.

roman-kashitsyn avatar Feb 22 '14 07:02 roman-kashitsyn