Re: [gentoo-portage-dev] [PATCH] Check for and report read-only filesystems

2014-01-13 Thread Mike Frysinger
On Friday 10 January 2014 22:07:52 Chris Reffett wrote:
 Attached is a patch to test if Portage is going to write to a
 read-only filesystem and print out the list of filesystems that need
 to be remounted RW. This leaves ${D} intact rather than having some
 files moved before hitting the RO filesystem. Fixes bug 378869. Since
 git.overlays.gentoo.org is down, I haven't had the chance to rebase
 this against latest, but I can resubmit if it doesn't cleanly apply.
 This is my first patch to the list, so I apologize if I didn't submit
 correctly.

please use `git send-email` to post patches.  attaching them makes things 
harder to review.

 --- /dev/null
 +++ b/pym/portage/util/checkwriteable.py

 + with open(/proc/mounts) as procmounts:

this won't work on non-Linux systems

also, as a general style thing, unless there's a real need for the var to have 
a full name, just use f or fp

 + roregex = re.compile(r'(\A|,)ro(\Z|,)')
 + for line in procmounts:
 + if roregex.search(line.split( )[3].strip()) 
 is not None:
 + romount = line.split( )[1].strip()
 + ro_filesystems.add(romount)

ad hoc parsing of /proc/mounts isn't a good thing.  at the risk of over 
engineering, there should be a func that would take care of expanding the 
mount paths into a list of namedcollections (one per mount).  then you simply 
walk it looking at its options.

 + for directory in dir_list:
 + for filesystem in ro_filesystems:
 + if re.match(filesystem, directory):
 + ro_filesystems_written.add(filesystem)

there's no need to use re and in fact you don't want to.  you're looking at 
wrong strings here, not regexes.
-mike


signature.asc
Description: This is a digitally signed message part.


Re: [gentoo-portage-dev] [PATCH] Check for and report read-only filesystems

2014-01-11 Thread Chris Reffett
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/11/2014 12:09 AM, Brian Dolbec wrote:
 On Fri, 2014-01-10 at 22:07 -0500, Chris Reffett wrote:
 Hi all, Attached is a patch to test if Portage is going to write
 to a read-only filesystem and print out the list of filesystems
 that need to be remounted RW. This leaves ${D} intact rather than
 having some files moved before hitting the RO filesystem. Fixes
 bug 378869. Since git.overlays.gentoo.org is down, I haven't had
 the chance to rebase this against latest, but I can resubmit if
 it doesn't cleanly apply. This is my first patch to the list, so
 I apologize if I didn't submit correctly.
 
 Chris Reffett
 
 
 yeah, patch looks good.
 
 Only thing I didn't like is the return 1  IS that suppose to be
 True or sys.exit() value?
 
 If that is what the module was using, then it's ok.  Personally I'm
 not a fan of using 0, 1 for False, True.
 
 But that will come later...
 
That was just following the style of the rest of the module, for
example a collision will return 1. This can be added to the stuff to
be fixed up in future patches list.

Chris Reffett
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iKUEARECAGYFAlLRU7VfFIAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl
bnBncC5maWZ0aGhvcnNlbWFuLm5ldEM2NzU5RjUyMDczREJDQkVDQTBDRkE1NERC
Nzk1QThBNDI2MTgzNTQACgkQ23laikJhg1QLjQCfSJSpacHoI/IQPS/o+NFJvP6q
d8YAmP+RmhoWwa3J1eRNk0BAxX1TtDg=
=a7If
-END PGP SIGNATURE-



[gentoo-portage-dev] [PATCH] Check for and report read-only filesystems

2014-01-10 Thread Chris Reffett
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi all,
Attached is a patch to test if Portage is going to write to a
read-only filesystem and print out the list of filesystems that need
to be remounted RW. This leaves ${D} intact rather than having some
files moved before hitting the RO filesystem. Fixes bug 378869. Since
git.overlays.gentoo.org is down, I haven't had the chance to rebase
this against latest, but I can resubmit if it doesn't cleanly apply.
This is my first patch to the list, so I apologize if I didn't submit
correctly.

Chris Reffett
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iKYEARECAGYFAlLQtYhfFIAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl
bnBncC5maWZ0aGhvcnNlbWFuLm5ldEM2NzU5RjUyMDczREJDQkVDQTBDRkE1NERC
Nzk1QThBNDI2MTgzNTQACgkQ23laikJhg1SCCgCfZIo57KiijmACnDTa2vC9UMAb
9YwAoIhnc/nHDcIXBbzF9Tie3eTJyWpH
=j/1A
-END PGP SIGNATURE-
From c464ac5e0007a087def9a96efea9653e508e57c1 Mon Sep 17 00:00:00 2001
From: Chris Reffett creff...@gentoo.org
Date: Fri, 10 Jan 2014 09:03:26 -0500
Subject: [PATCH] Test for read-only filesystems, bail out during preinst if
 there are any which will be written to and display a useful error message.
 Fixes bug 378869.

---
 pym/portage/dbapi/vartree.py   | 31 
 pym/portage/util/checkwriteable.py | 49 ++
 2 files changed, 80 insertions(+)
 create mode 100644 pym/portage/util/checkwriteable.py

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index ed62323..8ae7014 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -28,6 +28,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.util:apply_secpass_permissions,ConfigProtect,ensure_dirs,' + \
 		'writemsg,writemsg_level,write_atomic,atomic_ofstream,writedict,' + \
 		'grabdict,normalize_path,new_protect_filename',
+	'portage.util.checkwriteable:check_dirs_writeable',
 	'portage.util.digraph:digraph',
 	'portage.util.env_update:env_update',
 	'portage.util.listdir:dircache,listdir',
@@ -3508,6 +3509,8 @@ class dblink(object):
 		
 		This function does the following:
 		
+		calls check_dirs_writeable to verify that no files will be
+		written to read-only filesystems
 		calls self._preserve_libs if FEATURES=preserve-libs
 		calls self._collision_protect if FEATURES=collision-protect
 		calls doebuild(mydo=pkg_preinst)
@@ -3685,6 +3688,7 @@ class dblink(object):
 			eagain_error = False
 
 			myfilelist = []
+			mydirlist = []
 			mylinklist = []
 			paths_with_newlines = []
 			def onerror(e):
@@ -3717,6 +3721,9 @@ class dblink(object):
 	unicode_errors.append(new_parent[ed_len:])
 	break
 
+relative_path = parent[srcroot_len:]
+mydirlist.append(/ + relative_path)
+
 for fname in files:
 	try:
 		fname = _unicode_decode(fname,
@@ -3829,6 +3836,30 @@ class dblink(object):
 			for other in others_in_slot])
 		prepare_build_dirs(settings=self.settings, cleanup=cleanup)
 
+		# Check for read-only filesystems
+		rofilesystems = check_dirs_writeable(mydirlist)
+
+		if rofilesystems:
+			msg = _(One or more files installed to this package are 
+set to be installed to read-only filesystems. 
+Please mount the filesystems below read-write 
+and retry.)
+			msg = textwrap.wrap(msg, 70)
+			msg.append()
+			for f in rofilesystems:
+msg.append(\t%s % os.path.join(destroot,
+	f.lstrip(os.path.sep)))
+			msg.append()
+			self._elog(eerror, preinst, msg)
+
+			msg = _(Package '%s' NOT merged due to read-only file systems.) % \
+self.settings.mycpv
+			msg += _( If necessary, refer to your elog 
+messages for the whole content of the above message.)
+			msg = textwrap.wrap(msg, 70)
+			eerror(msg)
+			return 1
+
 		# check for package collisions
 		blockers = self._blockers
 		if blockers is None:
diff --git a/pym/portage/util/checkwriteable.py b/pym/portage/util/checkwriteable.py
new file mode 100644
index 000..9bd9056
--- /dev/null
+++ b/pym/portage/util/checkwriteable.py
@@ -0,0 +1,49 @@
+#-*- coding:utf-8 -*-
+# Copyright 2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from __future__ import unicode_literals
+
+import re
+
+from portage.util import writemsg
+from portage.localization import _
+
+
+def check_dirs_writeable(dir_list):
+	
+	Use /proc/mounts to check that no directories installed by the ebuild are set to
+	be installed to	a read-only filesystem
+
+	@param dir_list: Directories installed by the ebuild
+	@type dir_list: List
+	@return:
+	1. A list of filesystems which are both set to be written to and are mounted
+	read-only, may be empty.
+	
+	ro_filesystems = set()
+	ro_filesystems_written = set()
+
+	try:
+		with open(/proc/mounts) as procmounts:
+			roregex = re.compile(r'(\A|,)ro(\Z|,)')
+			for line in procmounts:
+if roregex.search(line.split( )[3].strip()) is not None:
+	romount = 

Re: [gentoo-portage-dev] [PATCH] Check for and report read-only filesystems

2014-01-10 Thread Brian Dolbec
On Fri, 2014-01-10 at 22:07 -0500, Chris Reffett wrote:
 Hi all,
 Attached is a patch to test if Portage is going to write to a
 read-only filesystem and print out the list of filesystems that need
 to be remounted RW. This leaves ${D} intact rather than having some
 files moved before hitting the RO filesystem. Fixes bug 378869. Since
 git.overlays.gentoo.org is down, I haven't had the chance to rebase
 this against latest, but I can resubmit if it doesn't cleanly apply.
 This is my first patch to the list, so I apologize if I didn't submit
 correctly.
 
 Chris Reffett


yeah, patch looks good.

Only thing I didn't like is the return 1  IS that suppose to be True or
sys.exit() value?

If that is what the module was using, then it's ok.  Personally I'm not
a fan of using 0, 1 for False, True.

But that will come later...


signature.asc
Description: This is a digitally signed message part