[gentoo-portage-dev] [PATCH] ro_checker: only check nearest parent (bug 547390)

[gentoo-portage-dev] [PATCH] ro_checker: only check nearest parent (bug 547390)

[Zac Medico]

The ro_checker code added in commit
47ef9a0969474f963dc8e52bfbbb8bc075e8d73c incorrectly asserts that all
parent directories be writable. Fix it to only assert that the nearest
parent directory be writable.

Fixes 47ef9a096947: ("Test for read-only filesystems, fixes bug 378869")
X-Gentoo-Bug: 547390
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=547390
---
 pym/portage/dbapi/vartree.py | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 1c0deab..c59d778 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -33,7 +33,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.util.env_update:env_update',
 	'portage.util.listdir:dircache,listdir',
 	'portage.util.movefile:movefile',
-	'portage.util.path:first_existing',
+	'portage.util.path:first_existing,iter_parents',
 	'portage.util.writeable_check:get_ro_checker',
 	'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
 	'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
@@ -3325,6 +3325,8 @@ class dblink(object):
 			showMessage = self._display_merge
 			stopmerge = False
 			collisions = []
+			dirs = set()
+			dirs_ro = set()
 			symlink_collisions = []
 			destroot = self.settings['ROOT']
 			showMessage(_(" %s checking %d files for package collisions\n") % \
@@ -3337,6 +3339,18 @@ class dblink(object):
 
 				dest_path = normalize_path(
 					os.path.join(destroot, f.lstrip(os.path.sep)))
+
+				parent = os.path.dirname(dest_path)
+				if parent not in dirs:
+					for x in iter_parents(parent):
+						if x in dirs:
+							break
+						dirs.add(x)
+						if os.path.isdir(x):
+							if not os.access(x, os.W_OK):
+								dirs_ro.add(x)
+							break
+
 				try:
 					dest_lstat = os.lstat(dest_path)
 				except EnvironmentError as e:
@@ -3410,7 +3424,7 @@ class dblink(object):
 							break
 					if stopmerge:
 						collisions.append(f)
-			return collisions, symlink_collisions, plib_collisions
+			return collisions, dirs_ro, symlink_collisions, plib_collisions
 
 	def _lstat_inode_map(self, path_iter):
 		"""
@@ -3759,7 +3773,6 @@ class dblink(object):
 			eagain_error = False
 
 			filelist = []
-			dirlist = []
 			linklist = []
 			paths_with_newlines = []
 			def onerror(e):
@@ -3792,13 +3805,6 @@ class dblink(object):
 					unicode_errors.append(new_parent[ed_len:])
 					break
 
-				relative_path = parent[srcroot_len:]
-				if len(relative_path) >= eprefix_len:
-					# Files are never installed outside of the prefix,
-					# therefore we skip the readonly filesystem check for
-					# parent directories of the prefix (see bug 544624).
-					dirlist.append(os.path.join(destroot, relative_path))
-
 				for fname in files:
 					try:
 						fname = _unicode_decode(fname,
@@ -3911,9 +3917,17 @@ class dblink(object):
 			for other in others_in_slot])
 		prepare_build_dirs(settings=self.settings, cleanup=cleanup)
 
+		# check for package collisions
+		blockers = self._blockers
+		if blockers is None:
+			blockers = []
+		collisions, dirs_ro, symlink_collisions, plib_collisions = \
+			self._collision_protect(srcroot, destroot,
+			others_in_slot + blockers, filelist, linklist)
+
 		# Check for read-only filesystems.
 		ro_checker = get_ro_checker()
-		rofilesystems = ro_checker(dirlist)
+		rofilesystems = ro_checker(dirs_ro)
 
 		if rofilesystems:
 			msg = _("One or more files installed to this package are "
@@ -3935,14 +3949,6 @@ class dblink(object):
 			eerror(msg)
 			return 1
 
-		# check for package collisions
-		blockers = self._blockers
-		if blockers is None:
-			blockers = []
-		collisions, symlink_collisions, plib_collisions = \
-			self._collision_protect(srcroot, destroot,
-			others_in_slot + blockers, filelist, linklist)
-
 		if symlink_collisions:
 			# Symlink collisions need to be distinguished from other types
 			# of collisions, in order to avoid confusion (see bug #409359).
-- 
2.3.5

[gentoo-portage-dev] [PATCH v2] ro_checker: only check nearest parent (bug 547390)

[Zac Medico]

The ro_checker code added in commit
47ef9a0969474f963dc8e52bfbbb8bc075e8d73c incorrectly asserts that all
parent directories be writable. Fix it to only assert that the nearest
parent directory be writable.

Fixes 47ef9a096947: ("Test for read-only filesystems, fixes bug 378869")
X-Gentoo-Bug: 547390
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=547390
---
[PATCH v2] fixes writeable_check to compare device numbers

 pym/portage/dbapi/vartree.py        | 44 +++++++++++++++++++++----------------
 pym/portage/util/writeable_check.py | 22 +++++++++++++++++--
 2 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 1c0deab..c59d778 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -33,7 +33,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.util.env_update:env_update',
 	'portage.util.listdir:dircache,listdir',
 	'portage.util.movefile:movefile',
-	'portage.util.path:first_existing',
+	'portage.util.path:first_existing,iter_parents',
 	'portage.util.writeable_check:get_ro_checker',
 	'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
 	'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
@@ -3325,6 +3325,8 @@ class dblink(object):
 			showMessage = self._display_merge
 			stopmerge = False
 			collisions = []
+			dirs = set()
+			dirs_ro = set()
 			symlink_collisions = []
 			destroot = self.settings['ROOT']
 			showMessage(_(" %s checking %d files for package collisions\n") % \
@@ -3337,6 +3339,18 @@ class dblink(object):
 
 				dest_path = normalize_path(
 					os.path.join(destroot, f.lstrip(os.path.sep)))
+
+				parent = os.path.dirname(dest_path)
+				if parent not in dirs:
+					for x in iter_parents(parent):
+						if x in dirs:
+							break
+						dirs.add(x)
+						if os.path.isdir(x):
+							if not os.access(x, os.W_OK):
+								dirs_ro.add(x)
+							break
+
 				try:
 					dest_lstat = os.lstat(dest_path)
 				except EnvironmentError as e:
@@ -3410,7 +3424,7 @@ class dblink(object):
 							break
 					if stopmerge:
 						collisions.append(f)
-			return collisions, symlink_collisions, plib_collisions
+			return collisions, dirs_ro, symlink_collisions, plib_collisions
 
 	def _lstat_inode_map(self, path_iter):
 		"""
@@ -3759,7 +3773,6 @@ class dblink(object):
 			eagain_error = False
 
 			filelist = []
-			dirlist = []
 			linklist = []
 			paths_with_newlines = []
 			def onerror(e):
@@ -3792,13 +3805,6 @@ class dblink(object):
 					unicode_errors.append(new_parent[ed_len:])
 					break
 
-				relative_path = parent[srcroot_len:]
-				if len(relative_path) >= eprefix_len:
-					# Files are never installed outside of the prefix,
-					# therefore we skip the readonly filesystem check for
-					# parent directories of the prefix (see bug 544624).
-					dirlist.append(os.path.join(destroot, relative_path))
-
 				for fname in files:
 					try:
 						fname = _unicode_decode(fname,
@@ -3911,9 +3917,17 @@ class dblink(object):
 			for other in others_in_slot])
 		prepare_build_dirs(settings=self.settings, cleanup=cleanup)
 
+		# check for package collisions
+		blockers = self._blockers
+		if blockers is None:
+			blockers = []
+		collisions, dirs_ro, symlink_collisions, plib_collisions = \
+			self._collision_protect(srcroot, destroot,
+			others_in_slot + blockers, filelist, linklist)
+
 		# Check for read-only filesystems.
 		ro_checker = get_ro_checker()
-		rofilesystems = ro_checker(dirlist)
+		rofilesystems = ro_checker(dirs_ro)
 
 		if rofilesystems:
 			msg = _("One or more files installed to this package are "
@@ -3935,14 +3949,6 @@ class dblink(object):
 			eerror(msg)
 			return 1
 
-		# check for package collisions
-		blockers = self._blockers
-		if blockers is None:
-			blockers = []
-		collisions, symlink_collisions, plib_collisions = \
-			self._collision_protect(srcroot, destroot,
-			others_in_slot + blockers, filelist, linklist)
-
 		if symlink_collisions:
 			# Symlink collisions need to be distinguished from other types
 			# of collisions, in order to avoid confusion (see bug #409359).
diff --git a/pym/portage/util/writeable_check.py b/pym/portage/util/writeable_check.py
index 445b2c2..0f7c08b 100644
--- a/pym/portage/util/writeable_check.py
+++ b/pym/portage/util/writeable_check.py
@@ -1,5 +1,5 @@
 #-*- coding:utf-8 -*-
-# Copyright 2014 Gentoo Foundation
+# Copyright 2014-2015 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 """
 Methods to check whether Portage is going to write to read-only filesystems.
@@ -13,6 +13,7 @@ from __future__ import unicode_literals
 
 import io
 import logging
+import os
 
 from portage import _encodings
 from portage.util import writemsg_level
@@ -68,7 +69,24 @@ def linux_ro_checker(dir_list):
 			level=logging.WARNING, noiselevel=-1)
 		return []
 
-	return set.intersection(ro_filesystems, set(dir_list))
+	ro_devs = set()
+	for x in ro_filesystems:
+		try:
+			ro_devs.add(os.stat(x).st_dev)
+		except OSError:
+			pass
+
+	ro_dirs = set()
+	for x in set(dir_list):
+		try:
+			dev = os.stat(x).st_dev
+		except OSError:
+			pass
+		else:
+			if dev in ro_devs:
+				ro_dirs.add(x)
+
+	return ro_dirs
 
 
 def empty_ro_checker(dir_list):
-- 
2.3.5

[gentoo-portage-dev] [PATCH v3] ro_checker: only check nearest parent (bug 547390)

[Zac Medico]

The ro_checker code added in commit
47ef9a0969474f963dc8e52bfbbb8bc075e8d73c incorrectly asserts that all
parent directories be writable. Fix it to only assert that the nearest
parent directory be writable.

Fixes 47ef9a096947: ("Test for read-only filesystems, fixes bug 378869")
X-Gentoo-Bug: 547390
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=547390
---
[PATCH v3] fixes writeable_check to return paths from mountinfo

 pym/portage/dbapi/vartree.py        | 44 +++++++++++++++++++++----------------
 pym/portage/util/writeable_check.py | 24 ++++++++++++++++++--
 2 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 1c0deab..c59d778 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -33,7 +33,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.util.env_update:env_update',
 	'portage.util.listdir:dircache,listdir',
 	'portage.util.movefile:movefile',
-	'portage.util.path:first_existing',
+	'portage.util.path:first_existing,iter_parents',
 	'portage.util.writeable_check:get_ro_checker',
 	'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
 	'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
@@ -3325,6 +3325,8 @@ class dblink(object):
 			showMessage = self._display_merge
 			stopmerge = False
 			collisions = []
+			dirs = set()
+			dirs_ro = set()
 			symlink_collisions = []
 			destroot = self.settings['ROOT']
 			showMessage(_(" %s checking %d files for package collisions\n") % \
@@ -3337,6 +3339,18 @@ class dblink(object):
 
 				dest_path = normalize_path(
 					os.path.join(destroot, f.lstrip(os.path.sep)))
+
+				parent = os.path.dirname(dest_path)
+				if parent not in dirs:
+					for x in iter_parents(parent):
+						if x in dirs:
+							break
+						dirs.add(x)
+						if os.path.isdir(x):
+							if not os.access(x, os.W_OK):
+								dirs_ro.add(x)
+							break
+
 				try:
 					dest_lstat = os.lstat(dest_path)
 				except EnvironmentError as e:
@@ -3410,7 +3424,7 @@ class dblink(object):
 							break
 					if stopmerge:
 						collisions.append(f)
-			return collisions, symlink_collisions, plib_collisions
+			return collisions, dirs_ro, symlink_collisions, plib_collisions
 
 	def _lstat_inode_map(self, path_iter):
 		"""
@@ -3759,7 +3773,6 @@ class dblink(object):
 			eagain_error = False
 
 			filelist = []
-			dirlist = []
 			linklist = []
 			paths_with_newlines = []
 			def onerror(e):
@@ -3792,13 +3805,6 @@ class dblink(object):
 					unicode_errors.append(new_parent[ed_len:])
 					break
 
-				relative_path = parent[srcroot_len:]
-				if len(relative_path) >= eprefix_len:
-					# Files are never installed outside of the prefix,
-					# therefore we skip the readonly filesystem check for
-					# parent directories of the prefix (see bug 544624).
-					dirlist.append(os.path.join(destroot, relative_path))
-
 				for fname in files:
 					try:
 						fname = _unicode_decode(fname,
@@ -3911,9 +3917,17 @@ class dblink(object):
 			for other in others_in_slot])
 		prepare_build_dirs(settings=self.settings, cleanup=cleanup)
 
+		# check for package collisions
+		blockers = self._blockers
+		if blockers is None:
+			blockers = []
+		collisions, dirs_ro, symlink_collisions, plib_collisions = \
+			self._collision_protect(srcroot, destroot,
+			others_in_slot + blockers, filelist, linklist)
+
 		# Check for read-only filesystems.
 		ro_checker = get_ro_checker()
-		rofilesystems = ro_checker(dirlist)
+		rofilesystems = ro_checker(dirs_ro)
 
 		if rofilesystems:
 			msg = _("One or more files installed to this package are "
@@ -3935,14 +3949,6 @@ class dblink(object):
 			eerror(msg)
 			return 1
 
-		# check for package collisions
-		blockers = self._blockers
-		if blockers is None:
-			blockers = []
-		collisions, symlink_collisions, plib_collisions = \
-			self._collision_protect(srcroot, destroot,
-			others_in_slot + blockers, filelist, linklist)
-
 		if symlink_collisions:
 			# Symlink collisions need to be distinguished from other types
 			# of collisions, in order to avoid confusion (see bug #409359).
diff --git a/pym/portage/util/writeable_check.py b/pym/portage/util/writeable_check.py
index 445b2c2..ac6c039 100644
--- a/pym/portage/util/writeable_check.py
+++ b/pym/portage/util/writeable_check.py
@@ -1,5 +1,5 @@
 #-*- coding:utf-8 -*-
-# Copyright 2014 Gentoo Foundation
+# Copyright 2014-2015 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 """
 Methods to check whether Portage is going to write to read-only filesystems.
@@ -13,6 +13,7 @@ from __future__ import unicode_literals
 
 import io
 import logging
+import os
 
 from portage import _encodings
 from portage.util import writemsg_level
@@ -68,7 +69,26 @@ def linux_ro_checker(dir_list):
 			level=logging.WARNING, noiselevel=-1)
 		return []
 
-	return set.intersection(ro_filesystems, set(dir_list))
+	ro_devs = {}
+	for x in ro_filesystems:
+		try:
+			ro_devs[os.stat(x).st_dev] = x
+		except OSError:
+			pass
+
+	ro_filesystems.clear()
+	for x in set(dir_list):
+		try:
+			dev = os.stat(x).st_dev
+		except OSError:
+			pass
+		else:
+			try:
+				ro_filesystems.add(ro_devs[dev])
+			except KeyError:
+				pass
+
+	return ro_filesystems
 
 
 def empty_ro_checker(dir_list):
-- 
2.3.5

Re: [gentoo-portage-dev] [PATCH v3] ro_checker: only check nearest parent (bug 547390)

[Brian Dolbec]

On Sun, 26 Apr 2015 21:30:28 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> The ro_checker code added in commit
> 47ef9a0969474f963dc8e52bfbbb8bc075e8d73c incorrectly asserts that all
> parent directories be writable. Fix it to only assert that the nearest
> parent directory be writable.
> 
> Fixes 47ef9a096947: ("Test for read-only filesystems, fixes bug
> 378869") X-Gentoo-Bug: 547390
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=547390
> ---
> [PATCH v3] fixes writeable_check to return paths from mountinfo
> 
>  pym/portage/dbapi/vartree.py        | 44
> +++++++++++++++++++++----------------
> pym/portage/util/writeable_check.py | 24 ++++++++++++++++++-- 2 files
> changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/pym/portage/dbapi/vartree.py
> b/pym/portage/dbapi/vartree.py index 1c0deab..c59d778 100644
> --- a/pym/portage/dbapi/vartree.py
> +++ b/pym/portage/dbapi/vartree.py
> @@ -33,7 +33,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
>  	'portage.util.env_update:env_update',
>  	'portage.util.listdir:dircache,listdir',
>  	'portage.util.movefile:movefile',
> -	'portage.util.path:first_existing',
> +	'portage.util.path:first_existing,iter_parents',
>  	'portage.util.writeable_check:get_ro_checker',
>  	'portage.util._dyn_libs.PreservedLibsRegistry:PreservedLibsRegistry',
>  	'portage.util._dyn_libs.LinkageMapELF:LinkageMapELF@LinkageMap',
> @@ -3325,6 +3325,8 @@ class dblink(object):
>  			showMessage = self._display_merge
>  			stopmerge = False
>  			collisions = []
> +			dirs = set()
> +			dirs_ro = set()
>  			symlink_collisions = []
>  			destroot = self.settings['ROOT']
>  			showMessage(_(" %s checking %d files for
> package collisions\n") % \ @@ -3337,6 +3339,18 @@ class
> dblink(object): 
>  				dest_path = normalize_path(
>  					os.path.join(destroot,
> f.lstrip(os.path.sep))) +
> +				parent = os.path.dirname(dest_path)
> +				if parent not in dirs:
> +					for x in
> iter_parents(parent):
> +						if x in dirs:
> +							break
> +						dirs.add(x)
> +						if os.path.isdir(x):
> +							if not
> os.access(x, os.W_OK):
> +
> dirs_ro.add(x)
> +							break
> +
>  				try:
>  					dest_lstat =
> os.lstat(dest_path) except EnvironmentError as e:
> @@ -3410,7 +3424,7 @@ class dblink(object):
>  							break
>  					if stopmerge:
>  						collisions.append(f)
> -			return collisions, symlink_collisions,
> plib_collisions
> +			return collisions, dirs_ro,
> symlink_collisions, plib_collisions 
>  	def _lstat_inode_map(self, path_iter):
>  		"""
> @@ -3759,7 +3773,6 @@ class dblink(object):
>  			eagain_error = False
>  
>  			filelist = []
> -			dirlist = []
>  			linklist = []
>  			paths_with_newlines = []
>  			def onerror(e):
> @@ -3792,13 +3805,6 @@ class dblink(object):
>  					unicode_errors.append(new_parent[ed_len:])
>  					break
>  
> -				relative_path = parent[srcroot_len:]
> -				if len(relative_path) >= eprefix_len:
> -					# Files are never installed
> outside of the prefix,
> -					# therefore we skip the
> readonly filesystem check for
> -					# parent directories of the
> prefix (see bug 544624).
> -
> dirlist.append(os.path.join(destroot, relative_path)) -
>  				for fname in files:
>  					try:
>  						fname =
> _unicode_decode(fname, @@ -3911,9 +3917,17 @@ class dblink(object):
>  			for other in others_in_slot])
>  		prepare_build_dirs(settings=self.settings,
> cleanup=cleanup) 
> +		# check for package collisions
> +		blockers = self._blockers
> +		if blockers is None:
> +			blockers = []
> +		collisions, dirs_ro, symlink_collisions,
> plib_collisions = \
> +			self._collision_protect(srcroot, destroot,
> +			others_in_slot + blockers, filelist,
> linklist) +
>  		# Check for read-only filesystems.
>  		ro_checker = get_ro_checker()
> -		rofilesystems = ro_checker(dirlist)
> +		rofilesystems = ro_checker(dirs_ro)
>  
>  		if rofilesystems:
>  			msg = _("One or more files installed to this
> package are " @@ -3935,14 +3949,6 @@ class dblink(object):
>  			eerror(msg)
>  			return 1
>  
> -		# check for package collisions
> -		blockers = self._blockers
> -		if blockers is None:
> -			blockers = []
> -		collisions, symlink_collisions, plib_collisions = \
> -			self._collision_protect(srcroot, destroot,
> -			others_in_slot + blockers, filelist,
> linklist) -
>  		if symlink_collisions:
>  			# Symlink collisions need to be
> distinguished from other types # of collisions, in order to avoid
> confusion (see bug #409359). diff --git
> a/pym/portage/util/writeable_check.py
> b/pym/portage/util/writeable_check.py index 445b2c2..ac6c039 100644
> --- a/pym/portage/util/writeable_check.py +++
> b/pym/portage/util/writeable_check.py @@ -1,5 +1,5 @@
>  #-*- coding:utf-8 -*-
> -# Copyright 2014 Gentoo Foundation
> +# Copyright 2014-2015 Gentoo Foundation
>  # Distributed under the terms of the GNU General Public License v2
>  """
>  Methods to check whether Portage is going to write to read-only
> filesystems. @@ -13,6 +13,7 @@ from __future__ import unicode_literals
>  
>  import io
>  import logging
> +import os
>  
>  from portage import _encodings
>  from portage.util import writemsg_level
> @@ -68,7 +69,26 @@ def linux_ro_checker(dir_list):
>  			level=logging.WARNING, noiselevel=-1)
>  		return []
>  
> -	return set.intersection(ro_filesystems, set(dir_list))
> +	ro_devs = {}
> +	for x in ro_filesystems:
> +		try:
> +			ro_devs[os.stat(x).st_dev] = x
> +		except OSError:
> +			pass
> +
> +	ro_filesystems.clear()
> +	for x in set(dir_list):
> +		try:
> +			dev = os.stat(x).st_dev
> +		except OSError:
> +			pass
> +		else:
> +			try:
> +				ro_filesystems.add(ro_devs[dev])
> +			except KeyError:
> +				pass
> +
> +	return ro_filesystems
>  
>  
>  def empty_ro_checker(dir_list):

looks fine

-- 
Brian Dolbec <dolsen>