public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
@ 2014-11-09 23:24 Zac Medico
  2014-11-10  4:58 ` [gentoo-portage-dev] " Duncan
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Zac Medico @ 2014-11-09 23:24 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

For unprivileged mode, if PORTAGE_DEPCACHEDIR is unset and the default
PORTAGE_DEPCACHEDIR setting does not refer to a writable directory
(or there are not sufficient permissions to create it), then
automatically make PORTAGE_DEPCACHEDIR relative to the current target
root (which should always be writable for unprivileged mode). Also, in
create_trees, propagate the automatically generated PORTAGE_DEPCACHEDIR
setting to the config instance that is instantiated for ROOT = "/".

The result is that unprivileged mode will get a writable
PORTAGE_DEPCACHEDIR by default, and the default can be overridden by
setting the PORTAGE_DEPCACHEDIR variable.

Fixes: 1364fcd89384 ("Support unprivileged mode for bug #433453.")
X-Gentoo-Bug: 433453
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=433453
---
 pym/portage/__init__.py              |  3 +++
 pym/portage/package/ebuild/config.py | 37 ++++++++++++++++++++++++++----------
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/pym/portage/__init__.py b/pym/portage/__init__.py
index 66bfeb0..d8046f3 100644
--- a/pym/portage/__init__.py
+++ b/pym/portage/__init__.py
@@ -570,6 +570,7 @@ def create_trees(config_root=None, target_root=None, trees=None, env=None,
 		env=env, eprefix=eprefix)
 	settings.lock()
 
+	depcachedir = settings.get('PORTAGE_DEPCACHEDIR')
 	trees._target_eroot = settings['EROOT']
 	myroots = [(settings['EROOT'], settings)]
 	if settings["ROOT"] == "/" and settings["EPREFIX"] == const.EPREFIX:
@@ -587,6 +588,8 @@ def create_trees(config_root=None, target_root=None, trees=None, env=None,
 			v = settings.get(k)
 			if v is not None:
 				clean_env[k] = v
+		if depcachedir is not None:
+			clean_env['PORTAGE_DEPCACHEDIR'] = depcachedir
 		settings = config(config_root=None, target_root="/",
 			env=clean_env, eprefix=None)
 		settings.lock()
diff --git a/pym/portage/package/ebuild/config.py b/pym/portage/package/ebuild/config.py
index 2ceb122..1ce5175 100644
--- a/pym/portage/package/ebuild/config.py
+++ b/pym/portage/package/ebuild/config.py
@@ -8,6 +8,7 @@ __all__ = [
 ]
 
 import copy
+import errno
 from itertools import chain
 import grp
 import logging
@@ -826,16 +827,6 @@ class config(object):
 			if "USE_ORDER" not in self:
 				self.backupenv["USE_ORDER"] = "env:pkg:conf:defaults:pkginternal:repo:env.d"
 
-			self.depcachedir = DEPCACHE_PATH
-			if portage.const.EPREFIX:
-				self.depcachedir = os.path.join(portage.const.EPREFIX,
-					DEPCACHE_PATH.lstrip(os.sep))
-
-			if self.get("PORTAGE_DEPCACHEDIR", None):
-				self.depcachedir = self["PORTAGE_DEPCACHEDIR"]
-			self["PORTAGE_DEPCACHEDIR"] = self.depcachedir
-			self.backup_changes("PORTAGE_DEPCACHEDIR")
-
 			if "CBUILD" not in self and "CHOST" in self:
 				self["CBUILD"] = self["CHOST"]
 				self.backup_changes("CBUILD")
@@ -898,6 +889,32 @@ class config(object):
 					self[var] = default_val
 				self.backup_changes(var)
 
+			self.depcachedir = self.get("PORTAGE_DEPCACHEDIR")
+			if self.depcachedir is None:
+				self.depcachedir = os.path.join(os.sep,
+					portage.const.EPREFIX, DEPCACHE_PATH.lstrip(os.sep))
+				if unprivileged and target_root != os.sep:
+					# In unprivileged mode, automatically make
+					# depcachedir relative to target_root if the
+					# default depcachedir is not writable.
+					current_dir = self.depcachedir
+					while current_dir != os.sep:
+						try:
+							st = os.stat(current_dir)
+						except OSError as e:
+							if errno == errno.ENOENT:
+								current_dir = \
+									os.path.dirname(current_dir)
+								continue
+						break
+
+					if not os.access(current_dir, os.W_OK):
+						self.depcachedir = os.path.join(eroot,
+							DEPCACHE_PATH.lstrip(os.sep))
+
+			self["PORTAGE_DEPCACHEDIR"] = self.depcachedir
+			self.backup_changes("PORTAGE_DEPCACHEDIR")
+
 			if portage._internal_caller:
 				self["PORTAGE_INTERNAL_CALLER"] = "1"
 				self.backup_changes("PORTAGE_INTERNAL_CALLER")
-- 
2.0.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [gentoo-portage-dev] Re: [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-09 23:24 [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
@ 2014-11-10  4:58 ` Duncan
  2014-11-10  5:28   ` Zac Medico
  2014-11-10 11:09 ` [gentoo-portage-dev] " Alexander Berntsen
  2014-11-10 20:48 ` [gentoo-portage-dev] [PATCH v2] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
  2 siblings, 1 reply; 13+ messages in thread
From: Duncan @ 2014-11-10  4:58 UTC (permalink / raw
  To: gentoo-portage-dev

Zac Medico posted on Sun, 09 Nov 2014 15:24:40 -0800 as excerpted:

> [...] then automatically make PORTAGE_DEPCACHEDIR relative to
> the current target root (which should always be writable for
> unprivileged mode).

Why?

Why does emerge --pretend need a writable target root in the first place, 
or it dies a horrible death (traceback)?

I keep root read-only by default, making it writable when I'm updating.  
When I'm simply doing an emerge --pretend, however, whether simply to 
satisfy my own curiosity or because I'm posting a reply to some other 
user where the output from emerge --pretend would be useful, why does 
emerge die a horrible death and traceback, when all I wanted was 
--pretend output that shouldn't be changing the target root at all and 
thus shouldn't /need/ a writable target root in the first place?

https://bugs.gentoo.org/show_bug.cgi?id=490732

FWIW, $PORTAGE_TMPDIR is writable, as is /run/lock (and thus
/var/run/lock).  In both tracebacks in the bug, it's a *.portage_lockfile 
that's not writable.  Why are those not in (possibly some subdir of)
/run/lock in the first place, or in $PORTAGE_TMPDIR, given the temporary 
nature of the files?  At least for --pretend.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] Re: [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-10  4:58 ` [gentoo-portage-dev] " Duncan
@ 2014-11-10  5:28   ` Zac Medico
  2014-11-10  6:06     ` Duncan
  0 siblings, 1 reply; 13+ messages in thread
From: Zac Medico @ 2014-11-10  5:28 UTC (permalink / raw
  To: gentoo-portage-dev

On 11/09/2014 08:58 PM, Duncan wrote:
> Zac Medico posted on Sun, 09 Nov 2014 15:24:40 -0800 as excerpted:
> 
>> [...] then automatically make PORTAGE_DEPCACHEDIR relative to
>> the current target root (which should always be writable for
>> unprivileged mode).
> 
> Why?

The "unprivileged mode" is similar to existing prefix support. The
"unprivileged mode" is basically useless unless your target root is
writable. Therefore, it's a sane assumption. It won't affect you unless
you use the new "unprivileged mode". If you do happen to use it, then
you will probably appreciate this patch.

As far as I can tell, the following discussion is about a bug that is
essentially unrelated to my proposed patch:

> Why does emerge --pretend need a writable target root in the first place, 
> or it dies a horrible death (traceback)?
> 
> I keep root read-only by default, making it writable when I'm updating.  
> When I'm simply doing an emerge --pretend, however, whether simply to 
> satisfy my own curiosity or because I'm posting a reply to some other 
> user where the output from emerge --pretend would be useful, why does 
> emerge die a horrible death and traceback, when all I wanted was 
> --pretend output that shouldn't be changing the target root at all and 
> thus shouldn't /need/ a writable target root in the first place?
> 
> https://bugs.gentoo.org/show_bug.cgi?id=490732
> 
> FWIW, $PORTAGE_TMPDIR is writable, as is /run/lock (and thus
> /var/run/lock).  In both tracebacks in the bug, it's a *.portage_lockfile 
> that's not writable.  Why are those not in (possibly some subdir of)
> /run/lock in the first place, or in $PORTAGE_TMPDIR, given the temporary 
> nature of the files?  At least for --pretend.

That bug should be easy to fix. We just need to handle the readonly case.
-- 
Thanks,
Zac


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [gentoo-portage-dev] Re: [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-10  5:28   ` Zac Medico
@ 2014-11-10  6:06     ` Duncan
  0 siblings, 0 replies; 13+ messages in thread
From: Duncan @ 2014-11-10  6:06 UTC (permalink / raw
  To: gentoo-portage-dev

Zac Medico posted on Sun, 09 Nov 2014 21:28:15 -0800 as excerpted:

> The "unprivileged mode" is similar to existing prefix support. The
> "unprivileged mode" is basically useless unless your target root is
> writable. Therefore, it's a sane assumption. It won't affect you unless
> you use the new "unprivileged mode". If you do happen to use it, then
> you will probably appreciate this patch.

I misinterpreted then, thinking unprivileged mode was simply running as a 
normal user.

Thanks.  (And good to see you back... with more help now. =:^)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-09 23:24 [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
  2014-11-10  4:58 ` [gentoo-portage-dev] " Duncan
@ 2014-11-10 11:09 ` Alexander Berntsen
  2014-11-10 18:21   ` Zac Medico
  2014-11-10 20:48 ` [gentoo-portage-dev] [PATCH v2] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
  2 siblings, 1 reply; 13+ messages in thread
From: Alexander Berntsen @ 2014-11-10 11:09 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 10/11/14 00:24, Zac Medico wrote:
> ir
> +					while current_dir != os.sep:
> +						try:
> +							st = os.stat(current_dir)
> +						except OSError as e:
> +							if errno == errno.ENOENT:
> +								current_dir = \
> +									os.path.dirname(current_dir)
> +								continue
> +						break
Can you think of a less terrible way of doing this in Python?

(Serious question, not snark!)
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRgnQYACgkQRtClrXBQc7X7IgD+OlJhVIsqEoeQkrpJhQOTypoU
uxOsZKj57wGO93cGwjQA/i5XAcFiQZz0ljLuZfSaG7rCXU5JtoMN7TDpi1HvF88O
=U/sJ
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-10 11:09 ` [gentoo-portage-dev] " Alexander Berntsen
@ 2014-11-10 18:21   ` Zac Medico
  2014-11-10 20:32     ` Alexander Berntsen
  0 siblings, 1 reply; 13+ messages in thread
From: Zac Medico @ 2014-11-10 18:21 UTC (permalink / raw
  To: gentoo-portage-dev

On 11/10/2014 03:09 AM, Alexander Berntsen wrote:
> On 10/11/14 00:24, Zac Medico wrote:
>> ir
>> +					while current_dir != os.sep:
>> +						try:
>> +							st = os.stat(current_dir)
>> +						except OSError as e:
>> +							if errno == errno.ENOENT:
>> +								current_dir = \
>> +									os.path.dirname(current_dir)
>> +								continue
>> +						break
> Can you think of a less terrible way of doing this in Python?

Well, you'll have to clarify what's so "terrible" about it. Note that I didn't
use os.path.isdir or similar because those functions hide all kinds of relevant
exceptions, such as EACCES.

If it's the continue and break that upset you, we can do it like this:

found_dir = False
while current_dir != os.sep and not found_dir:
	try:
		os.stat(current_dir)
		found_dir  = True
	except OSError:
		if errno == errno.ENOENT:
			current_dir = \
				os.path.dirname(current_dir)
		else:
			found_dir  = True

-- 
Thanks,
Zac


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-10 18:21   ` Zac Medico
@ 2014-11-10 20:32     ` Alexander Berntsen
  2014-11-11  0:17       ` Zac Medico
  0 siblings, 1 reply; 13+ messages in thread
From: Alexander Berntsen @ 2014-11-10 20:32 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 10/11/14 19:21, Zac Medico wrote:
> Well, you'll have to clarify what's so "terrible" about it. Note that I didn't
> use os.path.isdir or similar because those functions hide all kinds of relevant
> exceptions, such as EACCES.
I wish there were a more declarative way of expressing this in Python.
It's extremely imperative.

> If it's the continue and break that upset you, we can do it like this:
> 
> found_dir = False
> while current_dir != os.sep and not found_dir:
> 	try:
> 		os.stat(current_dir)
> 		found_dir  = True
> 	except OSError:
> 		if errno == errno.ENOENT:
> 			current_dir = \
> 				os.path.dirname(current_dir)
> 		else:
> 			found_dir  = True
That's a little better -- I'd prefer that. Thanks.
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRhIPkACgkQRtClrXBQc7WTpQD7BYE1+KdV8XG4h0JghIJ/dP4v
17isaVatmFdoSQdtwEsA/RXXK33wyWxR4yN0HDENtU5peRJPU/u/eRA/lobm31j9
=jLaK
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [gentoo-portage-dev] [PATCH v2] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-09 23:24 [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
  2014-11-10  4:58 ` [gentoo-portage-dev] " Duncan
  2014-11-10 11:09 ` [gentoo-portage-dev] " Alexander Berntsen
@ 2014-11-10 20:48 ` Zac Medico
  2 siblings, 0 replies; 13+ messages in thread
From: Zac Medico @ 2014-11-10 20:48 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

For unprivileged mode, if PORTAGE_DEPCACHEDIR is unset and the default
PORTAGE_DEPCACHEDIR setting does not refer to a writable directory
(or there are not sufficient permissions to create it), then
automatically make PORTAGE_DEPCACHEDIR relative to the current target
root (which should always be writable for unprivileged mode). Also, in
create_trees, propagate the automatically generated PORTAGE_DEPCACHEDIR
setting to the config instance that is instantiated for ROOT = "/".

The result is that unprivileged mode will get a writable
PORTAGE_DEPCACHEDIR by default, and the default can be overridden by
setting the PORTAGE_DEPCACHEDIR variable.

Fixes: 1364fcd89384 ("Support unprivileged mode for bug #433453.")
X-Gentoo-Bug: 433453
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=433453
---
This updated patch avoids use of continue and break in the loop that
checks for writable depcachedir, as requested by Alexander Berntsen.

 pym/portage/__init__.py              |  3 +++
 pym/portage/package/ebuild/config.py | 39 +++++++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/pym/portage/__init__.py b/pym/portage/__init__.py
index 66bfeb0..d8046f3 100644
--- a/pym/portage/__init__.py
+++ b/pym/portage/__init__.py
@@ -570,6 +570,7 @@ def create_trees(config_root=None, target_root=None, trees=None, env=None,
 		env=env, eprefix=eprefix)
 	settings.lock()
 
+	depcachedir = settings.get('PORTAGE_DEPCACHEDIR')
 	trees._target_eroot = settings['EROOT']
 	myroots = [(settings['EROOT'], settings)]
 	if settings["ROOT"] == "/" and settings["EPREFIX"] == const.EPREFIX:
@@ -587,6 +588,8 @@ def create_trees(config_root=None, target_root=None, trees=None, env=None,
 			v = settings.get(k)
 			if v is not None:
 				clean_env[k] = v
+		if depcachedir is not None:
+			clean_env['PORTAGE_DEPCACHEDIR'] = depcachedir
 		settings = config(config_root=None, target_root="/",
 			env=clean_env, eprefix=None)
 		settings.lock()
diff --git a/pym/portage/package/ebuild/config.py b/pym/portage/package/ebuild/config.py
index 2ceb122..a17a6b3 100644
--- a/pym/portage/package/ebuild/config.py
+++ b/pym/portage/package/ebuild/config.py
@@ -8,6 +8,7 @@ __all__ = [
 ]
 
 import copy
+import errno
 from itertools import chain
 import grp
 import logging
@@ -826,16 +827,6 @@ class config(object):
 			if "USE_ORDER" not in self:
 				self.backupenv["USE_ORDER"] = "env:pkg:conf:defaults:pkginternal:repo:env.d"
 
-			self.depcachedir = DEPCACHE_PATH
-			if portage.const.EPREFIX:
-				self.depcachedir = os.path.join(portage.const.EPREFIX,
-					DEPCACHE_PATH.lstrip(os.sep))
-
-			if self.get("PORTAGE_DEPCACHEDIR", None):
-				self.depcachedir = self["PORTAGE_DEPCACHEDIR"]
-			self["PORTAGE_DEPCACHEDIR"] = self.depcachedir
-			self.backup_changes("PORTAGE_DEPCACHEDIR")
-
 			if "CBUILD" not in self and "CHOST" in self:
 				self["CBUILD"] = self["CHOST"]
 				self.backup_changes("CBUILD")
@@ -898,6 +889,34 @@ class config(object):
 					self[var] = default_val
 				self.backup_changes(var)
 
+			self.depcachedir = self.get("PORTAGE_DEPCACHEDIR")
+			if self.depcachedir is None:
+				self.depcachedir = os.path.join(os.sep,
+					portage.const.EPREFIX, DEPCACHE_PATH.lstrip(os.sep))
+				if unprivileged and target_root != os.sep:
+					# In unprivileged mode, automatically make
+					# depcachedir relative to target_root if the
+					# default depcachedir is not writable.
+					current_dir = self.depcachedir
+					found_dir = False
+					while current_dir != os.sep and not found_dir:
+						try:
+							os.stat(current_dir)
+							found_dir = True
+						except OSError as e:
+							if e.errno == errno.ENOENT:
+								current_dir = \
+									os.path.dirname(current_dir)
+							else:
+								found_dir = True
+
+					if not os.access(current_dir, os.W_OK):
+						self.depcachedir = os.path.join(eroot,
+							DEPCACHE_PATH.lstrip(os.sep))
+
+			self["PORTAGE_DEPCACHEDIR"] = self.depcachedir
+			self.backup_changes("PORTAGE_DEPCACHEDIR")
+
 			if portage._internal_caller:
 				self["PORTAGE_INTERNAL_CALLER"] = "1"
 				self.backup_changes("PORTAGE_INTERNAL_CALLER")
-- 
2.0.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-10 20:32     ` Alexander Berntsen
@ 2014-11-11  0:17       ` Zac Medico
  2014-11-11  9:14         ` Alexander Berntsen
  0 siblings, 1 reply; 13+ messages in thread
From: Zac Medico @ 2014-11-11  0:17 UTC (permalink / raw
  To: gentoo-portage-dev

On 11/10/2014 12:32 PM, Alexander Berntsen wrote:
> On 10/11/14 19:21, Zac Medico wrote:
>> Well, you'll have to clarify what's so "terrible" about it. Note that I didn't
>> use os.path.isdir or similar because those functions hide all kinds of relevant
>> exceptions, such as EACCES.
> I wish there were a more declarative way of expressing this in Python.
> It's extremely imperative.

We could certainly express it in a way that doesn't involve any mutating
loop control variables, but ultimately that's going to lead to more
lines of code, and it will leave imperative programmers wondering why we
didn't choose a simpler and more succinct approach.

For example, we could create an class for iterating over the paths from
a given path down to the root directory. Then we could create a function
which selects the first element from that iterator that exists. Once the
class and function are implemented, their usage would be very succinct:

  first_parent = first_existing(iter_parents(path))
-- 
Thanks,
Zac


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-11  0:17       ` Zac Medico
@ 2014-11-11  9:14         ` Alexander Berntsen
  2014-11-15  5:19           ` Zac Medico
  0 siblings, 1 reply; 13+ messages in thread
From: Alexander Berntsen @ 2014-11-11  9:14 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 11/11/14 01:17, Zac Medico wrote:
> We could certainly express it in a way that doesn't involve any
> mutating loop control variables, but ultimately that's going to
> lead to more lines of code, and it will leave imperative
> programmers wondering why we didn't choose a simpler and more
> succinct approach.
> 
> For example, we could create an class for iterating over the paths
> from a given path down to the root directory. Then we could create
> a function which selects the first element from that iterator that
> exists. Once the class and function are implemented, their usage
> would be very succinct:
> 
> first_parent = first_existing(iter_parents(path))
I would greatly prefer this. But I suppose I'm in a minority. v2 of
the patch is fine by me.

- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRh04kACgkQRtClrXBQc7Wg7AD/WmncYIvR/f6OZ9W2mVfpgMmL
pZRD+68xWgWTdvatodYBAIX9VfX/0kINsmV9RhzumhLnHYE7LMz43nLy+yrekbxp
=H68V
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR
  2014-11-11  9:14         ` Alexander Berntsen
@ 2014-11-15  5:19           ` Zac Medico
  2014-11-15  7:21             ` [gentoo-portage-dev] [PATCH] unprivileged mode: use first_existing helper func Zac Medico
  0 siblings, 1 reply; 13+ messages in thread
From: Zac Medico @ 2014-11-15  5:19 UTC (permalink / raw
  To: gentoo-portage-dev

On 11/11/2014 01:14 AM, Alexander Berntsen wrote:
> On 11/11/14 01:17, Zac Medico wrote:
>> We could certainly express it in a way that doesn't involve any
>> mutating loop control variables, but ultimately that's going to
>> lead to more lines of code, and it will leave imperative
>> programmers wondering why we didn't choose a simpler and more
>> succinct approach.
> 
>> For example, we could create an class for iterating over the paths
>> from a given path down to the root directory. Then we could create
>> a function which selects the first element from that iterator that
>> exists. Once the class and function are implemented, their usage
>> would be very succinct:
> 
>> first_parent = first_existing(iter_parents(path))
> I would greatly prefer this. But I suppose I'm in a minority. v2 of
> the patch is fine by me.

Thanks. I eliminated one more backslash, and pushed it.

Now I'm thinking about splitting out a first_existing function so that I
can use it in portage.data._unprivileged_mode(), to determine if the
unprivileged root can be created. Maybe I'll do it with the iterator
approach that we've discussed, and we'll see what the consensus is.
-- 
Thanks,
Zac


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [gentoo-portage-dev] [PATCH] unprivileged mode: use first_existing helper func
  2014-11-15  5:19           ` Zac Medico
@ 2014-11-15  7:21             ` Zac Medico
  2014-11-17  8:12               ` Alexander Berntsen
  0 siblings, 1 reply; 13+ messages in thread
From: Zac Medico @ 2014-11-15  7:21 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

Split out a first_existing function, in order to improve logic related
to the _unprivileged_mode function so that it checks whether it's
possible to create the specified target root (instead of requiring that
the target root already exists).
---
 pym/portage/data.py                  | 20 +++++++++------
 pym/portage/package/ebuild/config.py | 23 ++++++-----------
 pym/portage/util/path.py             | 48 ++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 24 deletions(-)
 create mode 100644 pym/portage/util/path.py

diff --git a/pym/portage/data.py b/pym/portage/data.py
index 3e03eef..d9b36ee 100644
--- a/pym/portage/data.py
+++ b/pym/portage/data.py
@@ -8,6 +8,7 @@ import portage
 portage.proxy.lazyimport.lazyimport(globals(),
 	'portage.output:colorize',
 	'portage.util:writemsg',
+	'portage.util.path:first_existing',
 	'subprocess'
 )
 from portage.localization import _
@@ -94,14 +95,16 @@ def _get_global(k):
 		else:
 			# The config class has equivalent code, but we also need to
 			# do it here if _disable_legacy_globals() has been called.
-			eroot = os.path.join(os.environ.get('ROOT', os.sep),
-				portage.const.EPREFIX.lstrip(os.sep))
+			eroot_or_parent = first_existing(os.path.join(
+				os.environ.get('ROOT', os.sep),
+				portage.const.EPREFIX.lstrip(os.sep)))
 			try:
-				eroot_st = os.stat(eroot)
+				eroot_st = os.stat(eroot_or_parent)
 			except OSError:
 				pass
 			else:
-				unprivileged = _unprivileged_mode(eroot, eroot_st)
+				unprivileged = _unprivileged_mode(
+					eroot_or_parent, eroot_st)
 
 		v = 0
 		if uid == 0:
@@ -206,14 +209,15 @@ def _get_global(k):
 		else:
 			# The config class has equivalent code, but we also need to
 			# do it here if _disable_legacy_globals() has been called.
-			eroot = os.path.join(os.environ.get('ROOT', os.sep),
-				portage.const.EPREFIX.lstrip(os.sep))
+			eroot_or_parent = first_existing(os.path.join(
+				os.environ.get('ROOT', os.sep),
+				portage.const.EPREFIX.lstrip(os.sep)))
 			try:
-				eroot_st = os.stat(eroot)
+				eroot_st = os.stat(eroot_or_parent)
 			except OSError:
 				pass
 			else:
-				if _unprivileged_mode(eroot, eroot_st):
+				if _unprivileged_mode(eroot_or_parent, eroot_st):
 					if k == '_portage_grpname':
 						try:
 							grp_struct = grp.getgrgid(eroot_st.st_gid)
diff --git a/pym/portage/package/ebuild/config.py b/pym/portage/package/ebuild/config.py
index c7308a4..bf39487 100644
--- a/pym/portage/package/ebuild/config.py
+++ b/pym/portage/package/ebuild/config.py
@@ -48,6 +48,7 @@ from portage.util import ensure_dirs, getconfig, grabdict, \
 	grabdict_package, grabfile, grabfile_package, LazyItemsDict, \
 	normalize_path, shlex_split, stack_dictlist, stack_dicts, stack_lists, \
 	writemsg, writemsg_level, _eapi_cache
+from portage.util.path import first_existing
 from portage.util._path import exists_raise_eaccess, isdir_raise_eaccess
 from portage.versions import catpkgsplit, catsplit, cpv_getkey, _pkg_str
 
@@ -848,14 +849,16 @@ class config(object):
 				"PORTAGE_INST_UID": "0",
 			}
 
+			eroot_or_parent = first_existing(eroot)
 			unprivileged = False
 			try:
-				eroot_st = os.stat(eroot)
+				eroot_st = os.stat(eroot_or_parent)
 			except OSError:
 				pass
 			else:
 
-				if portage.data._unprivileged_mode(eroot, eroot_st):
+				if portage.data._unprivileged_mode(
+					eroot_or_parent, eroot_st):
 					unprivileged = True
 
 					default_inst_ids["PORTAGE_INST_GID"] = str(eroot_st.st_gid)
@@ -897,20 +900,8 @@ class config(object):
 					# In unprivileged mode, automatically make
 					# depcachedir relative to target_root if the
 					# default depcachedir is not writable.
-					current_dir = self.depcachedir
-					found_dir = False
-					while current_dir != os.sep and not found_dir:
-						try:
-							os.stat(current_dir)
-							found_dir = True
-						except OSError as e:
-							if e.errno == errno.ENOENT:
-								current_dir = os.path.dirname(
-									current_dir)
-							else:
-								found_dir = True
-
-					if not os.access(current_dir, os.W_OK):
+					if not os.access(first_existing(self.depcachedir),
+						os.W_OK):
 						self.depcachedir = os.path.join(eroot,
 							DEPCACHE_PATH.lstrip(os.sep))
 
diff --git a/pym/portage/util/path.py b/pym/portage/util/path.py
new file mode 100644
index 0000000..f77f30f
--- /dev/null
+++ b/pym/portage/util/path.py
@@ -0,0 +1,48 @@
+# Copyright 2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import errno
+
+from portage import os
+
+def first_existing(path):
+	"""
+	Returns the first existing path element, traversing from the given
+	path to the root directory. A path is considered to exists if lstat
+	either succeeds or raises an error other than ENOENT or ESTALE.
+
+	This can be particularly useful to check if there is permission to
+	create a particular file or directory, without actually creating
+	anything.
+
+	@param path: a filesystem path
+	@type path: str
+	@rtype: str
+	@return: the element that exists
+	"""
+	existing = False
+	for path in iter_parents(path):
+		try:
+			os.lstat(path)
+			existing = True
+		except OSError as e:
+			if e.errno not in (errno.ENOENT, errno.ESTALE):
+				existing = True
+
+		if existing:
+			return path
+
+	return os.sep
+
+def iter_parents(path):
+	"""
+	@param path: a filesystem path
+	@type path: str
+	@rtype: iterator
+	@return: an iterator which yields path and all parents of path,
+		ending with the root directory
+	"""
+	yield path
+	while path != os.sep:
+		path = os.path.dirname(path)
+		yield path
-- 
2.0.4



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] unprivileged mode: use first_existing helper func
  2014-11-15  7:21             ` [gentoo-portage-dev] [PATCH] unprivileged mode: use first_existing helper func Zac Medico
@ 2014-11-17  8:12               ` Alexander Berntsen
  0 siblings, 0 replies; 13+ messages in thread
From: Alexander Berntsen @ 2014-11-17  8:12 UTC (permalink / raw
  To: gentoo-portage-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Looks OK. Go ahead...

- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRprgYACgkQRtClrXBQc7WKYQD8DmeatGuSom5fxqXgR4VSzPve
+JMFmYlp/RNZ3Ya6YHgA/37BZ+Hoo4BfrHY9Owp98y4O6EACN3YVz6psmB/uJn5u
=Cfxl
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-11-17  8:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-09 23:24 [gentoo-portage-dev] [PATCH] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico
2014-11-10  4:58 ` [gentoo-portage-dev] " Duncan
2014-11-10  5:28   ` Zac Medico
2014-11-10  6:06     ` Duncan
2014-11-10 11:09 ` [gentoo-portage-dev] " Alexander Berntsen
2014-11-10 18:21   ` Zac Medico
2014-11-10 20:32     ` Alexander Berntsen
2014-11-11  0:17       ` Zac Medico
2014-11-11  9:14         ` Alexander Berntsen
2014-11-15  5:19           ` Zac Medico
2014-11-15  7:21             ` [gentoo-portage-dev] [PATCH] unprivileged mode: use first_existing helper func Zac Medico
2014-11-17  8:12               ` Alexander Berntsen
2014-11-10 20:48 ` [gentoo-portage-dev] [PATCH v2] unprivileged mode: generate PORTAGE_DEPCACHEDIR Zac Medico

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox