upgrade to mock-5.x breaks `csmock` about mock HOT 12 OPEN

1. creates a temporary file with the same ownership as the source file

I'm just surprised. It is a non-root process without any supplemental groups. Any intentions to change ownership must fail (at least on linux). So to me, it has been a bit weird the decision to prefer dos2unix failure over finishing the task (at least sed or awk would "just work" in place in the same environment).

What is completely unexpected here is that files in the build directory have GID set to something that the user running the build cannot set.

No doubt. We should do something about this.

from mock.

Thank you for the report!

dos2unix: Failed to change the owner and group of temporary output file samples/d2utmpCLdKHj: Operation not permitted

What ownership transition is being blocked here?

from mock.

It appears to me that Mock shouldn't "chown" the directory to kdudka:kdudka but kdudka:mock -> so you are able to read/write the files both in chroot and on host.

from mock.

% mock -r fedora-38-x86_64 -q --shell
<mock-chroot> sh-5.2# su - mockbuild
[mockbuild@e1860b412d5d45dbbd5fc18626102af9 ~]$ cd /builddir/build/BUILD/lynx2.9.0dev.10/
[mockbuild@e1860b412d5d45dbbd5fc18626102af9 lynx2.9.0dev.10]$ ls -l samples/lynx-demo.cfg 
-rw-r--r--. 1 mockbuild 1000 1093 Jul  8  2018 samples/lynx-demo.cfg

[mockbuild@e1860b412d5d45dbbd5fc18626102af9 lynx2.9.0dev.10]$ strace -e chown dos2unix samples/lynx-demo.cfg
chown("samples/d2utmp7brPHX", 1000, 1000) = -1 EPERM (Operation not permitted)
dos2unix: Failed to change the owner and group of temporary output file samples/d2utmp7brPHX: Operation not permitted
dos2unix: problems converting file samples/lynx-demo.cfg
+++ exited with 1 +++

GID 1000 is kdudka on my host. I am not sure why mock uses this group rather than mock with GID 135. I also do not think that recursively changing the ownership of /buildddir on each invocation of mock --chroot is a good idea.

from mock.

The mockbuild user is in the group 135 but not in the group 1000, which causes the failure (because non-root users can set group ownership only to groups they belong to):

[mockbuild@e1860b412d5d45dbbd5fc18626102af9 lynx2.9.0dev.10]$ id
uid=1000(mockbuild) gid=135(mock) groups=135(mock)

from mock.

chown("samples/d2utmp7brPHX", 1000, 1000) = -1 EPERM (Operation not permitted)

This call would probably never work, even with older Mock (setting group
ownership to GID 1000 was never possible). Actually, the
rpmbuild process never allowed chown() anything (rpmbuild was always run as
kdudka:mock inside the chroot - mapped as mockbuild:mock - and without any
supplemental groups).

I think that dos2unix feels some weird responsibility for fixing ownership
of processed files (which it is unlikely to have permissions for).

I also do not think that recursively changing the ownership of /buildddir on each invocation of mock --chroot

Good point. I think. Could we only do the chown before --rebuild? I need to re-read the git log again.

from mock.

I think that dos2unix feels some weird responsibility for fixing ownership
of processed files (which it is unlikely to have permissions for).

Which admittedly might be triggered by the change in Mock.

from mock.

I do not think that dos2unix does anything unexpected. It converts the file in-place atomically:

1. creates a temporary file with the same ownership as the source file
2. writes the converted data into the temporary file
3. renames the temporary file to the source file

What is completely unexpected here is that files in the build directory have GID set to something that the user running the build cannot set. The unexpected GID is set by mock --chroot in the updated version of mock. This did not break mock --rebuild because it does not use mock --chroot between %prep and %build but it did break csmock.

from mock.

The observed behavior of dos2unix is a known issue: https://access.redhat.com/solutions/3168671

from mock.

I think there are two things to improve in mock:

1. If it overrides GID of files in /builddir, it should always use the mock group.
2. It should not override UID/GID of files in /builddir on each invocation of mock --chroot.

from mock.

This is the hotfix I am deploying on OpenScanHub to make it work with mock-5.x:

--- a/py/mockbuild/buildroot.py
+++ b/py/mockbuild/buildroot.py
@@ -339,13 +339,13 @@ class Buildroot(object):
             self._cleanup_homedir()
             self._setup_build_dirs()

-        # (re)create users to ensure the uid/gid are up to date with config
-        # after doing 'dnf update', 'dnf builddep', etc.
-        self._make_users()
+            # (re)create users to ensure the uid/gid are up to date with config
+            # after doing 'dnf update', 'dnf builddep', etc.
+            self._make_users()

-        # Change owner of homdir tree if the root of it not owned
-        # by the current user
-        self.chown_home_dir()
+            # Change owner of homedir tree if the root of it is not owned
+            # by the current user
+            self.chown_home_dir()

         # mark the buildroot as initialized
         file_util.touch(self.make_chroot_path('.initialized'))

from mock.

Sorry, the previously shared hotfix did not work well. This is the hotfix I am deploying on OpenScanHub to make it work with mock-5.x:

--- a/py/mockbuild/buildroot.py
+++ b/py/mockbuild/buildroot.py
@@ -343,9 +343,10 @@ class Buildroot(object):
         # after doing 'dnf update', 'dnf builddep', etc.
         self._make_users()

-        # Change owner of homdir tree if the root of it not owned
-        # by the current user
-        self.chown_home_dir()
+        if prebuild:
+            # Change owner of homedir tree if the root of it is not owned
+            # by the current user
+            self.chown_home_dir()

         # mark the buildroot as initialized
         file_util.touch(self.make_chroot_path('.initialized'))

from mock.