Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symlinks can't be copied by install_subdir() since 0.47.0 #3914

Closed
didrocks opened this issue Jul 19, 2018 · 9 comments
Closed

Symlinks can't be copied by install_subdir() since 0.47.0 #3914

didrocks opened this issue Jul 19, 2018 · 9 comments

Comments

@didrocks
Copy link

didrocks commented Jul 19, 2018

If you have a dangling symlink (the symlink is referencing a path that is coming from another package on the system, so only resolved once installed on the system), in a directory, install_subdir is now reporting a stacktrace since the minstall.py rewrite (worked fine with 0.46, failing since 0.47.0 and still on 0.47.1).

For instance, you have:
data/ubuntu-wayland/applications/gnome-initial-setup.desktop -> ../../ubuntu/applications/gnome-initial-setup.desktop (only resolved on the installed system)

meson.build has:
install_subdir('data/ubuntu-wayland', install_dir: datadir) with datadir built as a package, so a destdir/prefix combo.

When building:

Installing subdir /home/didrocks/work/build-area/ubuntu-settings-18.04.6/data/ubuntu-wayland to /home/didrocks/work/build-area/ubuntu-settings-18.04.6/debian/tmp/usr/share/ubuntu-wayland
Traceback (most recent call last):
  File "/usr/bin/meson", line 29, in <module>
    sys.exit(mesonmain.main())
  File "/usr/lib/python3.6/dist-packages/mesonbuild/mesonmain.py", line 367, in main
    return run(sys.argv[1:], launcher)
  File "/usr/lib/python3.6/dist-packages/mesonbuild/mesonmain.py", line 281, in run
    return minstall.run(remaining_args)
  File "/usr/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 466, in run
    installer.do_install(datafilename)
  File "/usr/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 297, in do_install
    self.install_subdirs(d) # Must be first, because it needs to delete the old subtree.
  File "/usr/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 318, in install_subdirs
    self.do_copydir(d, src_dir, full_dst_dir, exclude, mode)
  File "/usr/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 281, in do_copydir
    self.do_copyfile(abs_src, abs_dst)
  File "/usr/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 196, in do_copyfile
    '{!r}'.format(from_file))
RuntimeError: Tried to install something that isn't a file:'/home/didrocks/work/build-area/ubuntu-settings-18.04.6/data/ubuntu-wayland/applications/gnome-initial-setup.desktop'

Shouldn't we either have a copy_symlink() or copy_file() be extended to support symlinks? (I see the code has a # FIXME: what about symlinks? as well ;))

@nirbheek
Copy link
Member

This is a regression and will be fixed for 0.47.2.

@nirbheek
Copy link
Member

@didrocks can you try the patch in #4014?

nirbheek added a commit that referenced this issue Aug 14, 2018
nirbheek added a commit that referenced this issue Aug 14, 2018
nirbheek added a commit that referenced this issue Aug 15, 2018
@3v1n0
Copy link
Contributor

3v1n0 commented Aug 16, 2018

Unforunately, this is not closed in the case where an install_umask is set, as happens with this test.

def test_install_subdir_symlinks_with_default_umask(self):
    '''
    Test that installation of broken symlinks works fine.
    https://github.com/mesonbuild/meson/issues/3914
    '''
    testdir = os.path.join(self.common_test_dir, '199 install_mode')
    subdir = os.path.join(testdir, 'sub1')
    curdir = os.getcwd()
    os.chdir(subdir)
    # Can't distribute broken symlinks in the source tree because it breaks
    # the creation of zipapps. Create it dynamically and run the test by
    # hand.
    src = '../../nonexistent.txt'
    os.symlink(src, 'invalid-symlink.txt')
    try:
        self.init(testdir)
        self.build()
        self.install()
        link = os.path.join(self.installdir, 'usr', 'share', 'sub1', 'invalid-symlink.txt')
        self.assertTrue(os.path.islink(link), msg=link)
        self.assertEqual(src, os.readlink(link))
        self.assertFalse(os.path.isfile(link), msg=link)
    finally:
        os.remove(os.path.join(subdir, 'invalid-symlink.txt'))
        os.chdir(curdir)

Causes

[0/1] Installing files.
Installing subdir /home/marco/Dev/tmp/meson/test cases/common/199 install_mode/sub1 to /tmp/tmpbtt9p_oi/install/usr/share/sub1
Installing /home/marco/Dev/tmp/meson/test cases/common/199 install_mode/sub1/second.dat to /tmp/tmpbtt9p_oi/install/usr/share/sub1
'/tmp/tmpbtt9p_oi/install/usr/share/sub1/second.dat': Unable to set owner 'root' and group None: Operation not permitted, ignoring...
Installing /home/marco/Dev/tmp/meson/test cases/common/199 install_mode/sub1/invalid-symlink.txt to /tmp/tmpbtt9p_oi/install/usr/share/sub1
Traceback (most recent call last):
  File "/home/marco/Dev/tmp/meson/meson.py", line 29, in <module>
    sys.exit(mesonmain.main())
  File "/home/marco/Dev/tmp/meson/mesonbuild/mesonmain.py", line 381, in main
    return run(sys.argv[1:], launcher)
  File "/home/marco/Dev/tmp/meson/mesonbuild/mesonmain.py", line 286, in run
    return minstall.run(remaining_args)
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 528, in run
    installer.do_install(datafilename)
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 352, in do_install
    self.install_subdirs(d) # Must be first, because it needs to delete the old subtree.
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 373, in install_subdirs
    self.do_copydir(d, src_dir, full_dst_dir, exclude, mode)
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 337, in do_copydir
    set_mode(abs_dst, install_mode, data.install_umask)
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 139, in set_mode
    set_chown(path, mode.owner, mode.group)
  File "/home/marco/Dev/tmp/meson/mesonbuild/minstall.py", line 129, in set_chown
    os.chown(path, _user, _group, dir_fd=dir_fd, follow_symlinks=follow_symlinks)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpbtt9p_oi/install/usr/share/sub1/invalid-symlink.txt'
FAILED: meson-install 
/usr/bin/python3 /home/marco/Dev/tmp/meson/meson.py install --no-rebuild
ninja: build stopped: subcommand failed.


----------------------------------------------------------------------
Ran 1 test in 0.504s

FAILED (errors=1)

3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 16, 2018
That's just useless when the linked file exists, while it would cause an error
otherwise.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 16, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, the best way was just reimplementing shutil.chown with just a small change
adding support to these parameters, without reinventing anything.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 16, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 16, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, the best way was just reimplementing shutil.chown with just a small change
adding support to these parameters, without reinventing anything.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 16, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, the best way was just reimplementing shutil.chown with just a small change
adding support to these parameters, without reinventing anything.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
This could happen when setting an default install mode but with broken
symlinks.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
This could happen when setting an default install mode but with broken
symlinks.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
This could happen when setting an default install mode but with broken
symlinks.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes mesonbuild#3914
3v1n0 added a commit to 3v1n0/meson that referenced this issue Aug 17, 2018
This could happen when setting an default install mode but with broken
symlinks.

Fixes mesonbuild#3914
@xnox
Copy link
Contributor

xnox commented Aug 22, 2018

@didrocks these fixes broke systemd installations!

nirbheek added a commit that referenced this issue Aug 23, 2018
nirbheek pushed a commit that referenced this issue Aug 23, 2018
It's only supported by few platforms when the linked file exists, while it
would cause an error otherwise.

In any case just implement this via an helper set_chmod function that will
handle the case where follow_symlinks is not supported by the platform
and will just not set any mod for the link itself (as it would otherwise
apply to the linked file).

Fixes #3914
nirbheek pushed a commit that referenced this issue Aug 23, 2018
Since we're supposed to call this for each installed path, we only should go
through what we've installed and not what this point to, as it might be
outside our scope or not existent.

To do this, since shutil.chown doesn't expose the follow_symlink that os.chown
has, we can temporarily replace os.chown with a lambda that acutually passes
all the values as we want them, and then restore it to the built-in functions.
Not the nicest way, but fixes the issue without having to reimplement what
shutil does.

Fixes #3914
nirbheek pushed a commit that referenced this issue Aug 23, 2018
This could happen when setting an default install mode but with broken
symlinks.

Fixes #3914
@didrocks
Copy link
Author

@xnox I guess you wanted to mention the fix author @3v1n0 ?

@3v1n0
Copy link
Contributor

3v1n0 commented Aug 27, 2018

@didrocks, @xnox nope, my change didn't break anything related to this (and in fact wasn't reverted) as it was just related to permissions/ownership of symlinks. The original fix for this (which @nirbheek did) was causing the systemd issue.
However, I agree the problem there was the systemd assumption and not meson change itself.

@xnox
Copy link
Contributor

xnox commented Aug 28, 2018

@didrocks @3v1n0 btw, i uploaded the fixed up patch to cosmic. Could you please double-check that whatever was affected for you two, is still working correctly in cosmic? (the thing that prompted upload of the first batch of cherrypicks)

@nirbheek
Copy link
Member

Note that this change broke systemd's optional installed tests, not systemd itself.

@Hi-Angel
Copy link
Contributor

Hi-Angel commented Sep 8, 2021

This seems to have regressed again. On meson 0.59.1 trying to install a subdir with symlinks results in:

Warning: trying to copy a symlink that points to a file. This will copy the file,
but this will be changed in a future version of Meson to copy the symlink as is. Please update your
build definitions so that it will not break when the change happens.

…and then instead of copying the symlink it copies the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants