Package is listed as installed even though permission error occured while extracting its contents

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Build opkg with parameters --with-opkglibdir and --with-opkgetcdir pointing 
to some dir accessible to your user, e.g. ~/my_sandbox.
2. Make a test package with paths inside set to some dir NOT accessible to your 
user, e.g. /usr or /lib. Let's assume there's only one /usr/test file inside.
3. Run opkg-cl install /path/to/test-package.opk. No sudo, no --offline-root, 
just run. You'll get an expected error:
Installing test-package (1.00) to root...
Configuring test-package.
Collected errors:
 * wfopen: /usr/test: Permission denied.

4. Now run opkg-cl info test-package. You'll get:
Package: test-package
Version: 1.00
Provides:
Status: install user installed
Architecture: x86_64
Installed-Time: 1327486049

5. Also run opkg-cl list-installed. You'll get:
test-package - 1.00

What is the expected output? What do you see instead?
I expected the package not to be listed as installed since the installation 
itself failed. However, the package info counts it as installed.

What version of the product are you using? On what operating system?
Latest at the moment (r635) on Linux Mint Debian Edition.

Please provide any additional information below.
I understand this isn't quite usual situation - the destination directory is 
not accessible but the opkg info/status dirs are. But you'll never know...

Original issue reported on code.google.com by paintitg...@gmail.com on 25 Jan 2012 at 10:18

[GoogleCodeExporter]

I was having a similar problem when updating Transmission from 2.42 to 2.60:

/share/transmission/web/style/transmission/images/toolbar-pause.png: No such 
file or directory.
 * wfopen: /share/transmission/web/style/transmission/Makefile.in: No such file or directory.
 * wfopen: /share/transmission/web/LICENSE: No such file or directory.
 * wfopen: /share/transmission/web/index.html: No such file or directory.
 * extract_archive: Cannot make dir /share/transmission/web/images/: No such file or directory.
 * wfopen: /share/transmission/web/images/Makefile.am: No such file or directory.
 * wfopen: /share/transmission/web/images/favicon.png: No such file or directory.
 * wfopen: /share/transmission/web/images/Makefile: No such file or directory.
 * wfopen: /share/transmission/web/images/favicon.ico: No such file or directory.
 * wfopen: /share/transmission/web/images/Makefile.in: No such file or directory.
 * wfopen: /share/transmission/web/images/webclip-icon.png: No such file or directory.
 * wfopen: /share/transmission/web/Makefile.in: No such file or directory.

What did work was setting the root installation directory to "/opt", as follows:
opkg-cl install -o /opt transmission-2.60-1.opk

"Upgrading transmission on root from 2.42-2 to 2.60-1..."

And now I have the new Transmission installed.

Original comment by arionkra...@gmail.com on 13 Jul 2012 at 4:51

[GoogleCodeExporter]

This doesn't really look related to this issue.

Original comment by paintitg...@gmail.com on 27 Jul 2012 at 11:44

[GoogleCodeExporter]

Could you try this again on the latest subversion sources and confirm whether 
it is still an issue? If so, could you send me the .opk files you use to test, 
would save me a bit of time...

If there's no reply within a month I'll try to test this out myself.

Original comment by paul.betafive on 3 Aug 2013 at 10:11

[GoogleCodeExporter]

Yes, still an issue at r739.
The output is exactly the same as almost two years ago.
Well, I've attached the .opk, don't know if it would help.

Original comment by paintitg...@gmail.com on 30 Oct 2013 at 11:11

Attachments:

• test-package.opk

[GoogleCodeExporter]

Ok, I'll take another look at this. I am currently replacing the current 
package extraction code in libbb/unarchive.c with the use of libarchive as the 
that project is well maintained and actively developed. That might solve this 
issue as errors should be better handled by libarchive.

Original comment by paul.betafive on 30 Oct 2013 at 11:41

• Changed state: Accepted
• Added labels: Milestone-0.3

[GoogleCodeExporter]

Relevant and still present in current code:

commit a1b131545aeb67349b066c1eece7e9bae903574c
Author: graham.gower <graham.gower@e8e0d7a0-c8d9-11dd-a880-a1081c7ac358>
Date:   Fri Dec 4 01:00:02 2009 +0000

    Ignore extraction errors, for now. Requested by Koen.

    As opkg has never failed from these errors in the past, many recipes in
    OpenEmbedded have come to rely on this behaviour. So proceed regardless of
    extraction errors for now, to give the OE folks a chance to fix broken
    packages first.

    git-svn-id: http://opkg.googlecode.com/svn/trunk@445 e8e0d7a0-c8d9-11dd-a880-a1081c7ac358

diff --git a/libbb/unarchive.c b/libbb/unarchive.c
index c3630b0..31c76d2 100644
--- a/libbb/unarchive.c
+++ b/libbb/unarchive.c
@@ -313,6 +313,7 @@ unarchive(FILE *src_stream, FILE *out_stream,
                        buffer = extract_archive(src_stream, out_stream,
                                        file_entry, extract_function,
                                        prefix, err);
+                       *err = 0; /* XXX: ignore extraction errors */
                        if (*err) {
                                free_headers(file_entry);
                                break;

Original comment by paul.betafive on 3 Dec 2013 at 11:45

[GoogleCodeExporter]

"Ignore extraction errors"? Excellent! :D
I wonder how many more quirks like this are hidden in the code.

Original comment by paintitg...@gmail.com on 4 Dec 2013 at 12:04

[GoogleCodeExporter]

I'm planning to replace the unarchive/unzip/deb_extract functions with calls 
out to libarchive which is actively maintained and supports a few things I'd 
like us to be able to use (POSIX format tarballs and optionally things like XZ 
compression). So this bit of cruft should disappear then.

It does mean I'll have to do a test build of lots of OpenEmbedded images 
without extraction errors being ignored before I release it.

Original comment by paul.betafive on 4 Dec 2013 at 2:37

[GoogleCodeExporter]

I guess I'll have to wait until then. This bug is a blocker for me. I can't 
rely on the software that pretends everything is fine when it's not so.

Original comment by paintitg...@gmail.com on 12 Dec 2013 at 10:27

[GoogleCodeExporter]

This should be fixed in the latest git master. Please could you give it a try.

Original comment by paul.betafive on 5 Feb 2014 at 6:19

[GoogleCodeExporter]

No it's not fixed in r746. Nothing changed.

Original comment by paintitg...@gmail.com on 20 Feb 2014 at 9:08

[GoogleCodeExporter]

We've moved opkg development to a git repo at git.yoctoproject.org, you'll need 
to try the latest sources from there. You'll also need libarchive installed 
along with it's development headers as we've used that to replace the older 
archive extraction code in opkg.

Original comment by paul.betafive on 20 Feb 2014 at 11:45

[GoogleCodeExporter]

Ok, I got the latest version from git, but it doesn't even want to install the 
package:

Collected errors:
 * extract_file_to_stream: Could not find the file 'control' in archive.
 * pkg_extract_control_file_to_stream: Failed to extract control file from package './test-package.opk'.
 * pkg_init_from_file: Failed to extract control file from ./test-package.opk.

What's wrong?
The package does contain DEBIAN/control file. Actually, this is still the same 
package, it's the one attached several posts above, so you can test this too.

Original comment by paintitg...@gmail.com on 24 Feb 2014 at 7:36

[GoogleCodeExporter]

Just a naive strcmp error: the file is name './control' in the archive which 
doesn't match 'control'. The test archive above also showed me a few other 
problems I hadn't seen in pkg_extract which weren't an issue for packages made 
using OpenEmbedded (such as ownership of the files in control.tar.gz).

Installing to an offline root '/tmp/opkg' as a non-root user I now get:

 * extract_all: Failed to extract file '/tmp/opkg/usr/' to disk.
 * extract_all: Archive error: Can't set user=1000/group=1000 for /tmp/opkg/usr
 * extract_all: Disk error: Can't set user=1000/group=1000 for /tmp/opkg/usr
 * pkg_extract_data_files_to_dir: Failed to extract data files from package '/home/pbarker/Downloads/test-package.opk'.
 * opkg_install_pkg: Failed to extract data files for test-package. Package debris may remain!
 * opkg_install_cmd: Cannot install package test-package.

I think that's expected, as a non-root user I can't set that ownership on files.

I don't have a system to hand on which I can test installing this to the actual 
root. Could you give the latest master a try?

Original comment by paul.betafive on 24 Feb 2014 at 12:42

[GoogleCodeExporter]

Yes, the latest git shows the same. It correctly processes the errors and 
doesn't mark the package as installed:

Installing test-package (0.42) on root.
Collected errors:
 * extract_all: Failed to extract file '/usr/' to disk.
 * extract_all: Archive error: Can't restore time
 * extract_all: Disk error: Can't restore time
 * pkg_extract_data_files_to_dir: Failed to extract data files from package './test-package.opk'.
 * opkg_install_pkg: Failed to extract data files for test-package. Package debris may remain!
 * opkg_install_cmd: Cannot install package test-package.

opkg-cl info test-package gives this:
Package: test-package
Version: 0.42
Status: install prefer,user not-installed
Architecture: x86_64

Should it be so? What is "install prefer"?

Also after this failed installation, the control file of test-package is left 
in /lib/opkg/info. Shouldn't it be removed (because the installation failed)?

Original comment by paintitg...@gmail.com on 4 Mar 2014 at 12:05

[GoogleCodeExporter]

There is currently no clean up if package installation fails. I think we want 
to leave the relevant control and list files in place but set the status flags 
to something which clearly indicates that the install has gone wrong. Simply 
removing this debris could cause major issues if the upgrade of an essential 
system package failed.

I think we should leave things in a consistent state where the user can tell 
from 'opkg info $pkg' that the install of $pkg failed and the user can run 
something like 'opkg remove $pkg' to remove the package debris.

Original comment by paul.betafive on 4 Mar 2014 at 1:32

[GoogleCodeExporter]

The question is, does the user need additional steps to remove something that 
failed to install, and does the user need this "history" of failed 
installations?

Original comment by paintitg...@gmail.com on 5 Mar 2014 at 1:47

[GoogleCodeExporter]

If an install dies half way through, it could have installed some files to root 
but not others. If this was a new package install and didn't overwrite 
anything, we would probably be safe in simply removing the files that had been 
written. However, if this was a package upgrade we may be left with some files 
from the new version and some from the old. I think in that case we're safer 
not removing anything automatically and giving the user the choice to reinstall 
the old version (if they have the package file cached somewhere, run "opkg 
install --force-reinstall $pkg_file.ipk") or to remove the entire package 
("opkg remove $pkg"). I don't think we can offer better options than that if an 
update goes awry.

As an example, say a libc upgrade fails. If we remove things automatically we 
could easily brick the system.

Original comment by paul.betafive on 5 Mar 2014 at 2:00

[GoogleCodeExporter]

Hmm, makes sense. Well then, I think some more explicit info should be added to 
status. The current cryptic "install prefer,user not-installed" string doesn't 
really tell user anything, doesn't indicate the error.

And if it was not just a failed install, but a failed upgrade, then some other 
info should be written there, indicating that the package is half-upgraded and 
probably broken.

Original comment by paintitg...@gmail.com on 6 Mar 2014 at 10:51

[GoogleCodeExporter]

The status line is a bit unclear to the user but is formed from 
pkg->state_want, pkg->state_flag and pkg->state_status. Reading 
http://manpages.ubuntu.com/manpages/lucid/man1/dpkg.1.html, I would say:

- pkg->state_want should be "install" (SW_INSTALL, meaning "The package is 
selected for installation.")

- pkg->state_flag should be "reinst-required" (SF_REINSTREQ, meaning "The 
package is  broken and requires reinstallation.")

- pkg->state_status should be "half-installed" (SS_HALF_INSTALLED, meaning "The 
installation of the package has been started, but not completed for some 
reason.")

After these flags have been set, I think we should print a clear error message 
telling the user what they should do next. ie. Run 'opkg remove $pkg' to remove 
the broken package or 'opkg install $pkg' to attempt to reinstall.

How does that sound?

Original comment by paul.betafive on 13 Mar 2014 at 7:09

[GoogleCodeExporter]

Maybe change "install" to "selected"? The "install" word itself doesn't explain 
that the package is actually selected for installation.

Other than that, I think it sounds fine.

Original comment by paintitg...@gmail.com on 20 Mar 2014 at 6:24

[GoogleCodeExporter]

I think we should keep the terms that dpkg uses and just make our output 
clearer. Something like

"Selected for:" with pkg->state_want
"Flags:" with pkg->state_flag
"Installation status:" with pkg->state_status

I'd like to avoid changing the actual terms unless we have our own explanation 
of these terms in a manual page. I have no experience writing manual pages 
myself and I'm lacking the time to learn so that would have to be handled by 
someone else.

Original comment by paul.betafive on 20 Mar 2014 at 2:30

[GoogleCodeExporter]

Though looking at the current code it may be easier to print the line "Status: 
want=%s flags=%s status=%s\n" with the appropriate substitutions. That would 
probably be clear enough.

Original comment by paul.betafive on 20 Mar 2014 at 6:25

[GoogleCodeExporter]

Changing the status line format breaks existing scripts so I'm not going to do 
it for now. I've made the change so that the line should read "Status: install 
reinst-required half-installed" which is about as clear as I can make things 
without breaking the format. I think that should close this out for now.

Original comment by paul.betafive on 22 Apr 2014 at 11:43

• Changed state: Fixed