[bug] Conan fails to import a read-only file twice

kdsx commented 4 years ago

Environment Details (include every applicable attribute)

Operating System+version: Arch Linux
Conan version: 1.21.2
Python version: 3.8

Steps to reproduce (Include if Applicable)

Create a package with a read-only file (I use OpenSSL/1.0.2q@conan/stable which contains read-only shared libs).
Import its files in another package.
```
from conans import ConanFile
```

class NxConan(ConanFile): settings = "os", "arch", "compiler", "build_type" requires = "OpenSSL/1.0.2q@conan/stable",

def configure(self):
    self.options["OpenSSL"].shared = True

def imports(self):
    self.copy("*.so*", "lib", "lib")

* Run conan install twice.

### Logs (Executed commands with output) (Include/Attach if Applicable)

First run - OK.

$ conan install . -if build Configuration: [settings] arch=x86_64 arch_build=x86_64 build_type=Release compiler=gcc compiler.libcxx=libstdc++ compiler.version=9 os=Linux os_build=Linux [options] [build_requires] [env]

conanfile.py: Installing package Requirements OpenSSL/1.0.2q@conan/stable from 'nx' - Cache zlib/1.2.11@conan/stable from 'conan-center' - Cache Packages OpenSSL/1.0.2q@conan/stable:160c0f76fcf1ad0d80837054bc0d59a5f4068208 - Cache zlib/1.2.11@conan/stable:176b7bd27fb0759a69f81f0746a288d1ce945b98 - Cache

zlib/1.2.11@conan/stable: Already installed! OpenSSL/1.0.2q@conan/stable: Already installed! conanfile.py: Generator txt created conanbuildinfo.txt conanfile.py: Generated conaninfo.txt conanfile.py: Generated graphinfo conanfile.py imports(): Copied 14 '.so' files conanfile.py imports(): Copied 2 '.0' files: libssl.so.1.0.0, libcrypto.so.1.0.0

Second run - FAIL

$ conan install . -if build Configuration: [settings] arch=x86_64 arch_build=x86_64 build_type=Release compiler=gcc compiler.libcxx=libstdc++ compiler.version=9 os=Linux os_build=Linux [options] [build_requires] [env]

conanfile.py: Installing package Requirements OpenSSL/1.0.2q@conan/stable from 'nx' - Cache zlib/1.2.11@conan/stable from 'conan-center' - Cache Packages OpenSSL/1.0.2q@conan/stable:160c0f76fcf1ad0d80837054bc0d59a5f4068208 - Cache zlib/1.2.11@conan/stable:176b7bd27fb0759a69f81f0746a288d1ce945b98 - Cache

zlib/1.2.11@conan/stable: Already installed! OpenSSL/1.0.2q@conan/stable: Already installed! conanfile.py: Generator txt created conanbuildinfo.txt conanfile.py: Generated conaninfo.txt conanfile.py: Generated graphinfo Traceback (most recent call last): File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/command.py", line 1947, in run method(args[0][1:]) File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/command.py", line 470, in install info = self._conan.install(path=args.path_or_reference, File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/conan_api.py", line 81, in wrapper return f(api, *args, *kwargs) File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/conan_api.py", line 553, in install deps_install(app=self.app, File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/manager.py", line 100, in deps_install run_imports(conanfile, install_folder) File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/importer.py", line 82, in run_imports conanfile.imports() File "/home/kds/tmp/example/conanfile.py", line 11, in imports self.copy(".so*", "lib", "lib") File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/importer.py", line 159, in call files = file_copier(pattern, src=src, links=True, ignore_case=ignore_case, File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/file_copier.py", line 80, in call fs = self._copy(src_folder, pattern, src, dst, links, ignore_case, excludes, File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/file_copier.py", line 99, in _copy copied_files = self._copy_files(files_to_copy, src, dst, keep_path, symlinks) File "/home/kds/.local/conan/lib/python3.8/site-packages/conans/client/file_copier.py", line 227, in _copy_files shutil.copy2(abs_src_name, abs_dst_name) File "/usr/lib/python3.8/shutil.py", line 426, in copy2 copyfile(src, dst, follow_symlinks=follow_symlinks) File "/usr/lib/python3.8/shutil.py", line 259, in copyfile with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst: PermissionError: [Errno 13] Permission denied: '/home/kds/tmp/example/build/lib/libcrypto.so.1.0.0'

ERROR: [Errno 13] Permission denied: '/home/kds/tmp/example/build/lib/libcrypto.so.1.0.0'

memsharded commented 4 years ago

Hi @kdsx

This is expected behavior. If the file is read-only, it is read-only. You should remove it, or change its mode before running a conan import again.

There are 2 different things here:

If that openssl package should actually have packaged files in RO mode. Why? Is it really necessary?
The error message could be definitely improved. This would be a UX feature/improvement. But the behavior wouldn't be changed.

The issue is that messing with read-only permissions is risky. If Conan removes the RO flag while importing, it might be doing something that the user didn't want. It is probable that the user wants to have those file imported in RO mode, because they are going to be used (there are users using conan import to deploy to systems). Accidentally overwriting a user file in userspace, that was marked as RO (maybe even manually by the user), seems also not the best thing.

So I would suggest to improve the error message to output the problem and the solution, but not to mess with the RO mode of user files. Wdyt?

kdsx commented 4 years ago

I don't think conan should remove RO flag (actually add write permission). Files must be imported with original permissions.

However conan knows the original file permissions when copying it. Also it knows that file was created by itself (it's listen in the manifest). Thus if nothing changed it can rewrite it, because there wasn't user intervention.

Of course if conan does not see a read-only file in the import manifest, it should not rewrite it.

Now it looks weird that conan does not have permissions to manage its own files.

Regarding OpenSSL: I don't know why it doesn't have +w flag on .so files. I don't think it was done intentionally.

P.S. Another idea is to add a parameter into import() function which allows to specify certain permissions for imported files. This partially solves the issue.

memsharded commented 4 years ago

I see. In theory, yes, the previous import manifest could be loaded, and the information propagated down to the FileImporter, that will pass it down the FileCopier, which could check if the file had been previously imported. This seems to be big changes to the functionality, which is also shared by the package() method self.copy(), so it should be done carefully to make sure it is not affecting, not making it slower.

The solution would be to clean the previously imported files first. There is already functionality for this, the conan imports --undo, so maybe it is easier to implement it this way, reusing the undo functionality. So my recommendation would be:

Locally remove the files, or use conan imports --undo to clean previously installed files, before importing again.
Report and check the reasons why the openssl package has -w on its files.
Consider adding a opt-in auto-undo feature for imports (could be made automatic and opt-out in Conan 2.0)

memsharded commented 10 months ago

imports have been removed in 2.0, closing this ticket as outdated

conan-io / conan