"package add" no longer works

[ttcoder]

Got this error today when packaging even though nothing changed (well something did, see below): Failed to parse package info date from package file: Bad data on this hrev47395 install.

Digging, I found out that package is choking on a StyledEdit attribute of the .PackageInfo file.. only now that it is combined with a Pe attribute. Indeed a few days ago I transitioned from using StyledEdit to exclusively using Pe, so I guess I'm now getting a double serving of "caret position" attributes on my textfiiles :-).

Anyway the workaround was simply to "reset" the .PackageInfo file, create a new one, free of text editor attributes. Filing a low-priority ticket in case the underlying reason matters though..

[ttcoder]

Here's a session showing the attribute contents:

/SamZulu/_BedRock/develop/TTSystems/TuneStacker/obj.x86> listattr .PackageInfo*
File: .PackageInfo
        Type       Size  Name                                
----------------------------------------------------------
 MIME String        11  "BEOS:TYPE"
      'info'       529  "pe-info"
File: .PackageInfo DEL
        Type       Size  Name                                
----------------------------------------------------------
 MIME String        11  "BEOS:TYPE"
      'RECT'        16  "StyledEdit-info"
      Int-32         4  "be:caret_position"
      'info'       529  "pe-info"

1100 bytes total in attributes.

(tried copying the first attr and doing the package: it succeeded)

/SamZulu/_BedRock/develop/TTSystems/TuneStacker/obj.x86> copyattr --name be:caret_position PackageInfo\ DEL .PackageInfo


(I then copied the *second* attr and doing the package: it failed!)

/SamZulu/_BedRock/develop/TTSystems/TuneStacker/obj.x86> copyattr --name StyledEdit-info .PackageInfo\ DEL .PackageInfo

(here's the attr contents if it matters)

/SamZulu/_BedRock/develop/TTSystems/TuneStacker/obj.x86> catattr StyledEdit-info .PackageInfo
.PackageInfo : 'RECT' : 0x000000:  40 a0 00 00 43 69 00 00 44 02 00 00 44 22 00 00   '@...Ci..D...D"..'

And here's the duo of packaging commands I run, the second of which being the one that fails with the mentionned error:

rm -f tunestacker-5.0.1.1-1-x86_gcc2.hpkg \
&& package create -q -C ./obj.x86 -b tunestacker-5.0.1.1-1-x86_gcc2.hpkg \
&& package add    -q -C ./obj.x86    tunestacker-5.0.1.1-1-x86_gcc2.hpkg TuneStacker data lib

EDIT: I think I remember that while digging, I tried to invoke the packaging in one go (no "create" followed by "add", just a "create" line) and it worked.. it only chokes when invoked as package-add. Curiouser and curiouser.

[ttcoder]

This has gotten worse now that I've updated to a recent hrev, package add no longer works at all, even if the .PackageInfo has no attribute at all. Here's a self-contained reproducible case, where cat creates a pristine info file.. If this does not get fixed soon I'm going to have to rewrite my packaging framework to use the alternate syntax, not looking forward to that :-(

mkdir -p obj3

cat << EOF > obj3/.PackageInfo
name proof
version 1.0-1
summary "test for packaging bug"
description "see above"
vendor ttcoder
packager ttcoder
copyrights 2014
licenses MIT
architecture any
provides { proof }
EOF

touch obj3/whatever


rm proof-1.0-1-any.hpkg 
package create -q -C ./obj3 -b proof-1.0-1-any.hpkg
package add    -q -C ./obj3    proof-1.0-1-any.hpkg whatever

Run the above in a newly created folder.

You'll get this:

> sh _create.sh 
Failed to parse package info data from package file: Bad data
> uname -a
Haiku shredder 1 hrev47896 Sep 22 2014 17:04:46 BePC x86 Haiku

[waddlesplash]

The "packager" field must be of the format "Name <something@…>" unless I'm mistaken...

[ttcoder]

Thanks, updated the self-contained testcase. To be clear, the "Failed to parse package info data from package file" intuitively would not seem to concern the .PackageInfo file at all, and it occurs during execution of the second line (package add, rather than package create). Appreciate if that can be fixed, I have to do my builds by hand..

[bonefish]

Replying to ttcoder:

Appreciate if that can be fixed, I have to do my builds by hand..

I don't think I'll get to do this anytime soon.

Anyway, the best strategy to create a package is to prepare a directory with all package content and use package create. This also has the advantage that you can easily use mimeset to generate the MIME DB entries to include in the package.

There's also package update, which is semantically similar (doesn't implicitly add directories, though) and, to my knowledge, does work.

[ttcoder]

update is not listed in usage.. and non-responsive if forcing its use anyway (at least in this hrev):

~/Desktop> package update my.hpkg blah
Usage: package <command> <command args>
Creates, inspects, or extracts a Haiku package.

Commands:
  add [ <options> ] <package> <entries>...

 (.. by contrast to "add" which responds:..)

~/Desktop> package add my.hpkg blah
Failed to open package file "my.hpkg": Read-only file system

Maybe it was added in the last couple weeks ?

Anyway I guess I'll move on to the 'mainstream' way of doing things, which seems cleaner in severals aspects indeed.. And this ticket will de facto become low priority, probably noone will/is missing "package add".

Appreciate your watching over this kind of tickets BTW thx

[bonefish]

Replying to ttcoder:

update is not listed in usage.. and non-responsive if forcing its use anyway (at least in this hrev):

Uh, sorry, I seem to have recalled that incorrectly. There's indeed only add, with the -f switch to replace existing files. Which we use in the build system for the update build action (e.g. jam -q @release-raw update libbe.so).

[mmlr]

I've investigated this and my conclusion is as follows:

When PackageFileHeapWriter tries to unwrite the last partial chunk in PackageFileHeapWriter::_UnwriteLastPartialChunk() it uses ReadData() which in turn calls into PackageFileHeapWriter::ReadAndDecompressChunk() with the index argument set to the last, partial, chunk (0 in case of the provided test case). Since it is the last chunk, it is assumed to be pending (as it would be once the last partial chunk was unwritten), so the pending buffer (which is yet to be filled) is copied to the uncompressed buffer instead of decompressing the actual last chunk. The pending buffer ends up containing nulls instead of the data of the last partial chunk.

Since the .PackageInfo is the only content of the package at this point, it fully resides in the last partial chunk and is therefore completely lost. Since the last chunk is lost in any case, using package add should always be corrupting the package, even when not failing to read the .PackageInfo (i.e. when there's enough additional data so that .PackageInfo doesn't reside in the last partial chunk).

I'm attaching a workaround that uses a flag to signify whether we are unwriting the last partial chunk so that PackageFileHeapWriter::ReadAndDecompressChunk() uncompresses the last chunk instead of returning the (yet unfilled) pending buffer. I can't really tell how to fix this properly.

[bonefish]

Thanks for your analysis, Michael! Your patch looks OK, but I'd rather avoid a flag to temporarily change the behavior of a method. Since _UnwriteLastPartialChunk() is meant to circumvent the usual mechanism of reading data, I'd rather not call ReadData() at all. For a single known chunk ReadDataToOutput() doesn't do much more than call ReadAndDecompressChunk() anyway, which in turn only calls ReadAndDecompressChunkData(). So calling the latter directly shouldn't result in much code duplication, but it would be clearer and even avoid a bit of overhead.

[mmlr]

Replying to bonefish:

So calling the latter directly shouldn't result in much code duplication, but it would be clearer and even avoid a bit of overhead.

Indeed, that sounds like a nice solution. If noone beats me to it, I'll come up with a new patch and push it tonight.

[mmlr]

Fixed in hrev47990. As per the suggestion above, _UnwriteLastPartialChunk() now uses ReadAndDecompressChunkData() directly. As it already has all the needed information, this was a really simple fix.

I also tested my hypothesis that this bug would always corrupt the last partial chunk of a package on update, which was indeed the case. Since the package created in the provided test case only consisted of a single chunk, it made the error more obvious. In other use cases the corruption would probably have gone unnoticed more easily.

Thanks to ttcoder for the nice test case and Ingo for the heads up on the elegant solution.