[gentoo-portage-dev] [PATCH] [REV] Added support for variable expansion in source command. wrt bug #490896

[gentoo-portage-dev] [PATCH] [REV] Added support for variable expansion in source command. wrt bug #490896

[Jesus Rivero (Neurogeek)]

[-- Attachment #1.1: Type: text/plain, Size: 432 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all,

In the previous patch, I didn't notice an error in the unittest I wrote for
the feature.
Attached is the correct patch, please disregard the previous one.

Cheers,

Jesus Rivero
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)

iEYEARECAAYFAlLS7oQACgkQdIssYB9vBoP5wgCgh0kIe28oKg5XECPkjoe36QaH
oY8An0iUVqvPi5cftWbpbrHO2nSsUcqz
=wZnl
-----END PGP SIGNATURE-----

[-- Attachment #1.2: Type: text/html, Size: 671 bytes --]

[-- Attachment #2: 0002-Added-support-for-variable-expansion-in-source-command.patch --]
[-- Type: application/octet-stream, Size: 4120 bytes --]

From 15e7c70ae8e0e7920f0b012c29f96fe42a4a6231 Mon Sep 17 00:00:00 2001
From: Jesus Rivero <neurogeek@gentoo.org>
Date: Sun, 12 Jan 2014 14:32:45 -0500
Subject: [CHANGE] Added support for variable expansion in source command
 argument in make.conf

Signed-off-by: Jesus Rivero <neurogeek@gentoo.org>
---
 cnf/make.conf.example.source_test        |  7 +++++++
 cnf/make.conf.example.source_test_after  |  8 ++++++++
 pym/portage/tests/util/test_getconfig.py | 18 ++++++++++++++++++
 pym/portage/util/__init__.py             |  7 ++++++-
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 cnf/make.conf.example.source_test
 create mode 100644 cnf/make.conf.example.source_test_after

diff --git a/cnf/make.conf.example.source_test b/cnf/make.conf.example.source_test
new file mode 100644
index 0000000..abacd35
--- /dev/null
+++ b/cnf/make.conf.example.source_test
@@ -0,0 +1,7 @@
+# Copyright 1999-2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+# Contains local system settings for Portage system
+
+# Test make.conf for variable expansion in 
+# source tokens.
+source "$PORTAGE_BASE_PATH/make.conf.example.source_test_after"
diff --git a/cnf/make.conf.example.source_test_after b/cnf/make.conf.example.source_test_after
new file mode 100644
index 0000000..bc45ef3
--- /dev/null
+++ b/cnf/make.conf.example.source_test_after
@@ -0,0 +1,8 @@
+# Copyright 1999-2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+# Contains local system settings for Portage system
+
+# Test make.conf for variable expansion in 
+# source tokens.
+# We should see this variable in getconfig result
+PASSES_SOURCING_TEST="True"
diff --git a/pym/portage/tests/util/test_getconfig.py b/pym/portage/tests/util/test_getconfig.py
index c7ab360..586a18e 100644
--- a/pym/portage/tests/util/test_getconfig.py
+++ b/pym/portage/tests/util/test_getconfig.py
@@ -8,6 +8,7 @@ from portage import _unicode_encode
 from portage.const import PORTAGE_BASE_PATH
 from portage.tests import TestCase
 from portage.util import getconfig
+from portage.exception import ParseError
 
 class GetConfigTestCase(TestCase):
 	"""
@@ -31,6 +32,23 @@ class GetConfigTestCase(TestCase):
 		for k, v in self._cases.items():
 			self.assertEqual(d[k], v)
 
+	def testGetConfigSourceLex(self):
+	 	base = os.path.join(PORTAGE_BASE_PATH, "cnf")
+		make_conf_file = os.path.join(base, 
+			'make.conf.example.source_test')
+
+		d = getconfig(make_conf_file, 
+			allow_sourcing=True, expand={"PORTAGE_BASE_PATH" : base})
+
+		# PASSES_SOURCING_TEST should exist in getconfig result
+		self.assertEqual("True", d['PASSES_SOURCING_TEST']) 
+
+		# With allow_sourcing : True and empty expand map, this should
+		# Throw a FileNotFound exception 
+		self.assertRaisesMsg("An empty expand map should throw an exception", 
+			ParseError, getconfig, *(make_conf_file,), 
+			**{'allow_sourcing' : True, 'expand' : {}})
+
 	def testGetConfigProfileEnv(self):
 		# Test the mode which is used to parse /etc/env.d and /etc/profile.env.
 
diff --git a/pym/portage/util/__init__.py b/pym/portage/util/__init__.py
index 24553da..0799487 100644
--- a/pym/portage/util/__init__.py
+++ b/pym/portage/util/__init__.py
@@ -593,8 +593,13 @@ class _getconfig_shlex(shlex.shlex):
 		shlex.shlex.__init__(self, **kwargs)
 		self.__portage_tolerant = portage_tolerant
 
+	def allow_sourcing(self, var_expand_map):
+		self.source = portage._native_string("source")
+		self.var_expand_map = var_expand_map
+
 	def sourcehook(self, newfile):
 		try:
+			newfile = varexpand(newfile, self.var_expand_map)
 			return shlex.shlex.sourcehook(self, newfile)
 		except EnvironmentError as e:
 			if e.errno == PermissionDenied.errno:
@@ -694,7 +699,7 @@ def getconfig(mycfg, tolerant=False, allow_sourcing=False, expand=True,
 			string.ascii_letters + "~!@#$%*_\:;?,./-+{}")
 		lex.quotes = portage._native_string("\"'")
 		if allow_sourcing:
-			lex.source = portage._native_string("source")
+			lex.allow_sourcing(expand_map)
 
 		while True:
 			key = _unicode_decode(lex.get_token())
-- 
1.8.3.2

Re: [gentoo-portage-dev] [PATCH] [REV] Added support for variable expansion in source command. wrt bug #490896

[Pavel Kazakov]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/12/2014 03:57 PM, Jesus Rivero (Neurogeek) wrote:
> Hi all,
>
> In the previous patch, I didn't notice an error in the unittest I
wrote for the feature.
> Attached is the correct patch, please disregard the previous one.
>
> Cheers,
>
> Jesus Rivero

Neurogeek,

Looks like we were working on the same patch, but I guess the patch is a
good exercise in getting familiar with the portage code :P

Overall it looks good. There were some complaints from git apply about
trivial spacing issues (trailing whitespace), and there's a space
included in indentation that causes python 3.3 to fail:
   
0002-Added-support-for-variable-expansion-in-source-commandd.patch:61:
space before tab in indent.

I'm not sure if it matters, but your code seems to do variable expansion
for the source command even if the 'expand' variable in the getconfig()
function is set to False (unless I'm wrong?).

All,

Although neurogeek already sent a patch, I figured I'd include mine as
well at the request of dol-sen :)

This patch does not include unit tests since it's meant to be more of a
review against neurogeek's code.

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

iQIcBAEBAgAGBQJS04RiAAoJENb1ecI556suiyYP/1X+CXK+WFoURBPUOlOyw9bN
jKYg5My7wcz07VnWrIkPiuQf3Pq3bj2ckMc54kiL59bMWQgMXjtZMJzyzfegza/t
Ks22+RYxmwdwU9OJ9prknFMLXdibEqT4zBk0MxHHfNeHqJtDM7j2Nc3Xcj+jP93R
zRFzFLjWjOeT4kCzUvmnEewjd9WlhycAhXB/MMoEQM/stAROv9sai7RhF+6EpXsE
XRP3ZagCdni5YA/Gz+eWZ59rihgo/8X1RLMzoK3MmPmh+LEi1Eop8NaHeTT4FumP
4ftr4mtc84qUpeo+pzzgi40Dse08oaHXCu5IX5lkVh0/Uu2vvVieTORGpLtDv9Mk
sK2qCZJ9JQU8l3pE2tRVkZ935IN/DxDoUCt5EosOB5OQ3UG6RkIP+hqNmcpkA6wC
OENrV5LCkAGzyuUNOCgqy4sL/rHWpSD+DqToc54vsnWTdkBCYO31AtwSKQ1xkihK
gp3Re+7Z6dA6KhjF5ZM6LOd5KyH6gbEH2EqtEuWeNiaeBB+5eYJuhLiqVOrmfgkl
nnatebh2DQum52OFDX4Q2M+6Cp0OHuYHUL0HxHcPRzmTnqBkwJtTxFoe2v3nQr5U
cTTrkM8t0TD7/CqHTo0JQQBleIEQGaI3jzHBaOS1DWxU4ZXaTbBrq8gng3H7RVW/
3mbx448SfEyeNGFiZqzE
=g+z5
-----END PGP SIGNATURE-----


[-- Attachment #2: 0001-Add-support-for-source-variable-expansion-bug-490896.patch --]
[-- Type: text/x-patch, Size: 1287 bytes --]

From 59f30b1901227031531d196bd0dcf59990c68ec3 Mon Sep 17 00:00:00 2001
From: Pavel Kazakov <nullishzero@gentoo.org>
Date: Sun, 12 Jan 2014 21:52:03 -0800
Subject: [PATCH] Add support for source variable expansion, bug #490896

---
 pym/portage/util/__init__.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pym/portage/util/__init__.py b/pym/portage/util/__init__.py
index 24553da..555ac76 100644
--- a/pym/portage/util/__init__.py
+++ b/pym/portage/util/__init__.py
@@ -592,9 +592,12 @@ class _getconfig_shlex(shlex.shlex):
 	def __init__(self, portage_tolerant=False, **kwargs):
 		shlex.shlex.__init__(self, **kwargs)
 		self.__portage_tolerant = portage_tolerant
+		self.expand_map = None
 
 	def sourcehook(self, newfile):
 		try:
+			if self.expand_map is not None:
+				newfile = varexpand(newfile, self.expand_map)
 			return shlex.shlex.sourcehook(self, newfile)
 		except EnvironmentError as e:
 			if e.errno == PermissionDenied.errno:
@@ -695,6 +698,8 @@ def getconfig(mycfg, tolerant=False, allow_sourcing=False, expand=True,
 		lex.quotes = portage._native_string("\"'")
 		if allow_sourcing:
 			lex.source = portage._native_string("source")
+		if expand:
+			lex.expand_map = expand_map
 
 		while True:
 			key = _unicode_decode(lex.get_token())
-- 
1.8.3.2

Re: [gentoo-portage-dev] [PATCH] [REV] Added support for variable expansion in source command. wrt bug #490896

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 464 bytes --]

On Sunday 12 January 2014 18:57:43 Jesus Rivero (Neurogeek) wrote:
> In the previous patch, I didn't notice an error in the unittest I wrote for
> the feature.
> Attached is the correct patch, please disregard the previous one.

please use standard git tags which means the subject line would be:
	[PATCH v2] Added support for ....

also please use `git send-email` rather than attaching files.  it makes things 
harder to review when you attach.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-portage-dev] [PATCH] [REV] Added support for variable expansion in source command. wrt bug #490896

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 225 bytes --]

>  cnf/make.conf.example.source_test        |  7 +++++++
>  cnf/make.conf.example.source_test_after  |  8 ++++++++

i don't think test files belong in cnf/.  keep them in the same dir as the test 
that uses them.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]