Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 574422 - dev-python/pydecomp: Please add Gentoo/FreeBSD's tar support.
Summary: dev-python/pydecomp: Please add Gentoo/FreeBSD's tar support.
Status: RESOLVED OBSOLETE
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Development (show other bugs)
Hardware: All FreeBSD
: Normal normal (vote)
Assignee: Portage Tools Team
URL:
Whiteboard:
Keywords: InVCS
Depends on:
Blocks: 363577
  Show dependency tree
 
Reported: 2016-02-11 11:56 UTC by Yuta SATOH
Modified: 2019-10-12 12:17 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
bad sample patch (pydecomp-0.1-fbsd.patch,2.57 KB, patch)
2016-02-11 12:08 UTC, Yuta SATOH
Details | Diff
patch for pydecomp (pydecomp-9999.patch,7.84 KB, patch)
2016-03-21 15:16 UTC, Yuta SATOH
Details | Diff
patch for catalyst-9999 (catalyst-pydecomp-v2.patch,3.78 KB, patch)
2016-03-21 15:19 UTC, Yuta SATOH
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta SATOH 2016-02-11 11:56:13 UTC
FreeBSD of tar does not support --xattrs and -I options.
Compression and decompression can be overwritten by configuration of catalyst.


CONTENTS_DEFINITIONS can not be overwritten by configuration file.
In the case of FreeBSD, please change xattr option is disabled and drop pyxz, lbzip2.


FYI,
Set in /etc/catalyst.conf
compression_mode="bzip2"
decompressor_search_order="bzip2 xz gzip tar"
Comment 1 Yuta SATOH 2016-02-11 12:08:27 UTC
Created attachment 425190 [details, diff]
bad sample patch
Comment 2 Brian Dolbec (RETIRED) gentoo-dev 2016-02-11 15:17:25 UTC
No, it's not a bad patch.

It was suggested to change the xattr variants in the compress/decompress to a more generic options that can be mixed in from the consumer app side.  Very much like your patch for the contents.
Comment 3 Yuta SATOH 2016-02-13 01:50:54 UTC
I was confirm that can use other tools by using --use-compress-program instead of -I.

Decompression and list archive requires care.
--use-compress-program "lbzip2 -d" is required on FreeBSD.

* Decompress

# tar -I 'lbzip2' -xpf /var/tmp/catalyst/builds/default/stage3-amd64-fbsd-10.2-20160211t.tar.bz2 -C /var/tmp/portage/tmpfs
tar: Error inclusion pattern: Failed to open 'lbzip2'

# tar --use-compress-program lbzip2 -xpf /var/tmp/catalyst/builds/default/stage3-amd64-fbsd-10.2-20160211t.tar.bz2 -C /var/tmp/portage/tmpfs
tar: Error opening archive: Child process exited with status 1

# tar --use-compress-program lbzip2 -d -xpf /var/tmp/catalyst/builds/default/stage3-amd64-fbsd-10.2-20160211t.tar.bz2 -C /var/tmp/portage/tmpfs
Usage:
  List:    tar -tf <archive-filename>
  Extract: tar -xf <archive-filename>
  Create:  tar -cf <archive-filename> [filenames...]
  Help:    tar --help

# tar --use-compress-program "lbzip2 -d" -xpf /var/tmp/catalyst/builds/default/stage3-amd64-fbsd-10.2-20160211t.tar.bz2 -C /var/tmp/portage/tmpfs


* Compress

# tar --use-compress-program lbzip2 -cf ~/test-lbzip2.tar.bz2 -C /var/tmp/portage/tmpfs .

* List archive

# tar --use-compress-program "lbzip2 -d" -tvf /var/tmp/catalyst/builds/default/stage3-amd64-fbsd-10.2-20160211t.tar.bz2


FYI,
You will be able to reproduce this result by using the bsdtar on Linux.

$ equery b /usr/bin/tar
 * Searching for /usr/bin/tar ...
app-arch/libarchive-3.1.2-r3 (/usr/bin/tar -> bsdtar)
app-arch/libarchive-3.1.2-r3 (/usr/bin/bsdtar)
Comment 4 SpanKY gentoo-dev 2016-02-13 08:23:37 UTC
(In reply to Yuta SATOH from comment #3)

--use-compress-program is a good solution

forcing --xattrs to only when the host OS is Linux seems wrong.  it depends on tar behavior, not the host OS.  seems like bug 538654 would address this.

so i guess you don't actually need to disable lbzip2 from the list.
Comment 5 Brian Dolbec (RETIRED) gentoo-dev 2016-02-13 08:50:56 UTC
OK, I'm going to work on this in the morning, try to get it all ironed out this weekend.  But now, it's past my bedtime.
Comment 6 Brian Dolbec (RETIRED) gentoo-dev 2016-03-20 03:43:48 UTC
I've implemented a similar solution.  if os.uname()[0] is Linux, then it sets teh default other_options to [] instead of the current xattr options.

I've also added compressor_options to the catalyst spec file so they are fully configurable.

Please re-emerge both pydecomp-9999 and catalys-9999 to test.

New spec options:

compressor_options: --foo --bar
compressor_arc:  # <== squashfs specific
Comment 7 Yuta SATOH 2016-03-21 15:16:29 UTC
Created attachment 428712 [details, diff]
patch for pydecomp

(In reply to Brian Dolbec from comment #6)
> I've implemented a similar solution.  if os.uname()[0] is Linux, then it
> sets teh default other_options to [] instead of the current xattr options.
> 
> I've also added compressor_options to the catalyst spec file so they are
> fully configurable.
> 
> Please re-emerge both pydecomp-9999 and catalys-9999 to test.
> 
> New spec options:
> 
> compressor_options: --foo --bar
> compressor_arc:  # <== squashfs specific

I tested it, but did not work properly on Gentoo/FreeBSD...
Patch for pydecomp will fix the following problems on FreeBSD.

* lbzip2 and pixz requires -d option for decompression/list (Comment 3).
* --xattrs is included in CONTENTS_DEFINITIONS, but it is not supported on FreeBSD (Comment 0).
Comment 8 Yuta SATOH 2016-03-21 15:19:01 UTC
Created attachment 428714 [details, diff]
patch for catalyst-9999

Catalyst part.
Adding new options, decomp_opt and list_xattrs_opt.
In addition, fixed some of issues.

* catalyst/targets/snapshot.py
snapshot should also allow comp_prog.

Error Log,
# catalyst -d -C target=snapshot version_stamp=20160321 
tar: Couldn't open lbzip2: No such file or directory
21 Mar 2016 21:19:45 JST: ERROR   : snapshot.py:run: Snapshot compression failure

* catalyst/base/stagebase.py
When decompression, KeyError: 'comp_prog' occurs.
Added 'comp_prog': self.settings["comp_prog"], to unpack_info=.

Error Log,
21 Mar 2016 23:31:05 JST: ERROR   : Exception running action sequence unpack
Traceback (most recent call last):
  File "/usr/lib/python3.4/site-packages/catalyst/base/stagebase.py", line 1420, in run
    getattr(self, x)()
  File "/usr/lib/python3.4/site-packages/catalyst/base/stagebase.py", line 787, in unpack
    if not self.decompressor.extract(unpack_info):
  File "/usr/lib/python3.4/site-packages/DeComp/compress.py", line 170, in _extract
    return self._run(infodict)
  File "/usr/lib/python3.4/site-packages/DeComp/compress.py", line 198, in _run
    success = func(infodict)
  File "/usr/lib/python3.4/site-packages/DeComp/compress.py", line 300, in _common
    opts = ' '.join(cmdargs) %(cmdinfo)
KeyError: 'comp_prog'


And, when compressing, comp_prog is not set.
Added comp_prog to self.compressor.

Error Log,
22 Mar 2016 00:04:51 JST: NOTICE  : Creating stage tarball... mode: lbzip2
tar: Couldn't open lbzip2: No such file or directory
22 Mar 2016 00:04:51 JST: WARNING : Couldn't create stage tarball: /var/tmp/catalyst/builds/default/stage2-amd64-fbsd-10.2-20160211t.tar.bz2


* catalyst/defaults.py
Change the default value of the following.
Currently, I think that option of XATTR are provided by compressor_options.

DECOMPRESSOR_XATTR_SEARCH_ORDER --> DECOMPRESSOR_SEARCH_ORDER
"compression_mode": 'lbzip2_x', --> "compression_mode": 'lbzip2',
Comment 9 Brian Dolbec (RETIRED) gentoo-dev 2016-03-21 17:37:14 UTC
First, thank you for testing.  I see a couple of my mistakes that you fixed. Only one of them I already knew about.  The other I accidentally removed the comp_prog parameter and left the logger=log one which was the one I was going to remove.  I added it for easier initial testing along with more log messages.

I do have some questions...

What was the purpose of...  "bsd": " -d", '"lbzip2%(decomp_opt)s"' and the join/split change from a space to \t

-        _cmd.extend((' '.join(args)
+        _cmd.extend(('\t'.join(args)

-                    ).split()
+                    ).split('\t')


This does not make sense to me.  If it is just so there isn't a null string member of the args list... That is a no brainer to remove null strings later.  In fact I did add that already.

I would much prefer to keep the original format.  eg:

"bsd": "-d", "lbzip2", "%(decomp_opt)s"

and remove any resulting null strings later.

Also, there may be a potential problem later for a subprocess call. it doesn't like when it gets a mix of list elements and space separated strings as part of those elements.  It either has to be one (space separated) string or a pure (no spaces) list.  Using tab separation and a potential mix of space separations does not seem like a solid plan.
Comment 10 Yuta SATOH 2016-03-23 10:48:06 UTC
(In reply to Brian Dolbec from comment #9)
> First, thank you for testing.  I see a couple of my mistakes that you fixed.
> Only one of them I already knew about.  The other I accidentally removed the
> comp_prog parameter and left the logger=log one which was the one I was
> going to remove.  I added it for easier initial testing along with more log
> messages.
> 
> I do have some questions...
> 
> What was the purpose of...  "bsd": " -d", '"lbzip2%(decomp_opt)s"' and the
> join/split change from a space to \t
> 
> -        _cmd.extend((' '.join(args)
> +        _cmd.extend(('\t'.join(args)
> 
> -                    ).split()
> +                    ).split('\t')
> 
> 
> This does not make sense to me.  If it is just so there isn't a null string
> member of the args list... That is a no brainer to remove null strings
> later.  In fact I did add that already.
> 
> I would much prefer to keep the original format.  eg:
> 
> "bsd": "-d", "lbzip2", "%(decomp_opt)s"
> 
> and remove any resulting null strings later.
> 
> Also, there may be a potential problem later for a subprocess call. it
> doesn't like when it gets a mix of list elements and space separated strings
> as part of those elements.  It either has to be one (space separated) string
> or a pure (no spaces) list.  Using tab separation and a potential mix of
> space separations does not seem like a solid plan.

Thanks for review.
I'm not familiar with python. I cannot attach a better patch...

bsdtar, extract and list archive, argument given to --use-compress-program must be quoted "lbzip2 -d" or "pixz -d" (Please see comment 3).

In the case of the original, it would be split.
['tar', '--use-compress-program', 'lbzip2', '-d', '-tvf', '/var/tmp/catalyst/snapshots/portage-20160321.tar.bz2']

When you change the code, argument is set appropriately.
['tar', '', '--use-compress-program', 'lbzip2 -d', '-tvf', '/var/tmp/catalyst/snapshots/portage-20160321.tar.bz2']

Note,
I have confirmed this value by the addition of print (_cmd) to contents.py.
+       print (_cmd)
        try:
            proc = Popen(_cmd, stdout=PIPE, stderr=PIPE)
Comment 11 SpanKY gentoo-dev 2016-04-06 16:41:31 UTC
imo we should not use or support executing shell pipelines.  we should be forcing everything to be sane argv's as then we don't have to worry about quoting issues.  that means the calls to Popen and such use lists.  which is to say, we're already using shell=False (the default), so that part is fixed.

the issue that bubbles up is that we do ' '.join(args).split().  the reason is to be lazy i guess with the dict interpolation.  it should instead be something like:

 _cmd = [cmd]
 settings = {
     'source': source,
     ...
 }
 _cmd.extend(x % settings for x in args)

i *think* that'll DTRT here.
Comment 12 Michael 'veremitz' Everitt 2018-08-04 07:28:29 UTC
Picking this one up to show some 'love' to pydecomp, and add another feature.

@vapier, did you conclude anything further about issues from #c11 ?

@Yuta, are you still able to test patches to this? Or would it be worth our while spinning up a Gentoo-BSD VM somewhere ... :/

@dol-sen can we update the Component definition here pls?!

TIA! :]
Comment 13 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-10-12 12:17:33 UTC
G/FBSD is dead.