public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598
@ 2014-10-26 11:12 zmedico
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " zmedico
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: zmedico @ 2014-10-26 11:12 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

From: Zac Medico <zmedico@gentoo.org>

This includes numerous logic adjustments that are needed to support
protected symlinks. The show_diff function now supports arbitrary
file types. For example, a diff between two symlinks looks like this:

-SYM: /foo/bar -> baz
+SYM: /foo/bar -> blah

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
 bin/etc-update | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 85 insertions(+), 8 deletions(-)

diff --git a/bin/etc-update b/bin/etc-update
index 7ac6f0b..3c03d7a 100755
--- a/bin/etc-update
+++ b/bin/etc-update
@@ -51,11 +51,15 @@ do_mv_ln() {
 	local src=${@:$(( $# - 1 )):1}
 	local dst=${@:$(( $# - 0 )):1}
 
-	if [[ -L ${dst} ]] ; then #330221
+	if [[ ! -L ${src} && -L ${dst} ]] ; then #330221
 		local lfile=$(readlink "${dst}")
 		[[ ${lfile} == /* ]] || lfile="${dst%/*}/${lfile}"
 		echo " Target is a symlink; replacing ${lfile}"
 		dst=${lfile}
+	elif [[ -d ${dst} && ! -L ${dst} ]] ; then
+		# If ${dst} is a directory, do not move the file
+		# inside of it if this fails.
+		rmdir "${dst}" || return
 	fi
 
 	mv "${opts[@]}" "${src}" "${dst}"
@@ -115,6 +119,24 @@ scan() {
 					continue 2
 				fi
 			done
+			if [[ -L ${file} ]] ; then
+				if [[ -L ${live_file} && \
+					$(readlink "${live_file}") == $(readlink "${file}") ]]
+				then
+					rm -f "${file}"
+					continue
+				fi
+				if [[ "${ofile:10}" != "${rfile:10}" ]] ||
+				   [[ ${opath} != ${rpath} ]]
+				then
+					: $(( ++count ))
+					echo "${live_file}" > "${TMP}"/files/${count}
+				fi
+				echo "${cfg_file}" >> "${TMP}"/files/${count}
+				ofile="${rfile}"
+				opath="${rpath}"
+				continue
+			fi
 			if [[ ! -f ${file} ]] ; then
 				${QUIET} || echo "Skipping non-file ${file} ..."
 				continue
@@ -124,7 +146,9 @@ scan() {
 			   [[ ${opath} != ${rpath} ]]
 			then
 				MATCHES=0
-				if [[ ${eu_automerge} == "yes" ]] ; then
+				if ! [[ -f ${cfg_file} && -f ${live_file} ]] ; then
+					MATCHES=0
+				elif [[ ${eu_automerge} == "yes" ]] ; then
 					if [[ ! -e ${cfg_file} || ! -e ${live_file} ]] ; then
 						MATCHES=0
 					else
@@ -377,17 +401,48 @@ do_file() {
 
 show_diff() {
 	clear
-	local file1=$1 file2=$2
+	local file1=$1 file2=$2 files=("$1" "$2") \
+		diff_files=() file i tmpdir
+
+	if [[ -L ${file1} && ! -L ${file2} &&
+		-f ${file1} && -f ${file2} ]] ; then
+		# If a regular file replaces a symlink to a regular file, then
+		# show the diff between the regular files (bug #330221).
+		diff_files=("${file1}" "${file2}")
+	else
+		for i in 0 1 ; do
+			if [[ ! -L ${files[$i]} && -f ${files[$i]} ]] ; then
+				diff_files[$i]=${files[$i]}
+				continue
+			fi
+			[[ -n ${tmpdir} ]] || \
+				tmpdir=$(mktemp -d "${TMP}/symdiff-XXX")
+			diff_files[$i]=${tmpdir}/${i}
+			if [[ -L ${files[$i]} ]] ; then
+				echo "SYM: ${file1} -> $(readlink "${files[$i]}")" > \
+					"${diff_files[$i]}"
+			elif [[ -d ${files[$i]} ]] ; then
+				echo "DIR: ${file1}" > "${diff_files[$i]}"
+			elif [[ -p ${files[$i]} ]] ; then
+				echo "FIF: ${file1}" > "${diff_files[$i]}"
+			else
+				echo "DEV: ${file1}" > "${diff_files[$i]}"
+			fi
+		done
+	fi
+
 	if [[ ${using_editor} == 0 ]] ; then
 		(
 			echo "Showing differences between ${file1} and ${file2}"
-			diff_command "${file1}" "${file2}"
+			diff_command "${diff_files[0]}" "${diff_files[1]}"
 		) | ${pager}
 	else
 		echo "Beginning of differences between ${file1} and ${file2}"
-		diff_command "${file1}" "${file2}"
+		diff_command "${diff_files[0]}" "${diff_files[1]}"
 		echo "End of differences between ${file1} and ${file2}"
 	fi
+
+	[[ -n ${tmpdir} ]] && rm -rf "${tmpdir}"
 }
 
 do_cfg() {
@@ -395,14 +450,14 @@ do_cfg() {
 	local ofile=$2
 	local -i my_input=0
 
-	until (( my_input == -1 )) || [ ! -f "${file}" ] ; do
+	until (( my_input == -1 )) || [[ ! -f ${file} && ! -L ${file} ]] ; do
 		if [[ "${OVERWRITE_ALL}" == "yes" ]] && ! user_special "${ofile}"; then
 			my_input=1
 		elif [[ "${DELETE_ALL}" == "yes" ]] && ! user_special "${ofile}"; then
 			my_input=2
 		else
 			show_diff "${ofile}" "${file}"
-			if [[ -L ${file} ]] ; then
+			if [[ -L ${file} && ! -L ${ofile} ]] ; then
 				cat <<-EOF
 
 					-------------------------------------------------------------
@@ -461,6 +516,19 @@ do_merge() {
 	local ofile="${2}"
 	local mfile="${TMP}/${2}.merged"
 	local -i my_input=0
+
+	if [[ -L ${file} && -L ${ofile} ]] ; then
+		echo "Both files are symlinks, so they will not be merged."
+		return 0
+	elif [[ ! -f ${file} ]] ; then
+		echo "Non-regular file cannot be merged: ${file}"
+		return 0
+	elif [[ ! -f ${ofile} ]] ; then
+		echo "Non-regular file cannot be merged: ${ofile}"
+		return 0
+	fi
+
+
 	echo "${file} ${ofile} ${mfile}"
 
 	if [[ -e ${mfile} ]] ; then
@@ -533,9 +601,18 @@ do_distconf() {
 	for (( count = 0; count <= 9999; ++count )) ; do
 		suffix=$(printf ".dist_%04i" ${count})
 		efile="${ofile}${suffix}"
-		if [[ ! -f ${efile} ]] ; then
+		if [[ ! -f ${efile} && ! -L ${efile} ]] ; then
 			mv ${mv_opts} "${file}" "${efile}"
 			break
+		elif [[ -L ${efile} && -L ${file} ]] ; then
+			if [[ $(readlink "${efile}") == $(readlink "${file}") ]] ; then
+				# replace identical copy
+				mv "${file}" "${efile}"
+				break
+			fi
+		elif [[ -L ${efile} || -L ${file} ]] ; then
+			# not the same file types
+			continue
 		elif diff_command "${file}" "${efile}" &> /dev/null; then
 			# replace identical copy
 			mv "${file}" "${efile}"
-- 
2.0.4



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

* [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: symlink support for bug #485598
  2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
@ 2014-10-26 11:12 ` zmedico
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, " zmedico
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: zmedico @ 2014-10-26 11:12 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

From: Zac Medico <zmedico@gentoo.org>

This includes numerous logic adjustments that are needed to support
protected symlinks. The new diff_mixed function is used for diffs
between arbitrary file types. For example, a diff between two symlinks
looks like this:

-SYM: /foo/bar -> baz
+SYM: /foo/bar -> blah

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
 bin/dispatch-conf            |  40 +++++++------
 pym/portage/dispatch_conf.py | 137 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 139 insertions(+), 38 deletions(-)

diff --git a/bin/dispatch-conf b/bin/dispatch-conf
index 6d2ae94..80dafd6 100755
--- a/bin/dispatch-conf
+++ b/bin/dispatch-conf
@@ -15,15 +15,15 @@ from __future__ import print_function
 
 from stat import ST_GID, ST_MODE, ST_UID
 from random import random
-import atexit, re, shutil, stat, sys
+import atexit, io, re, functools, shutil, sys
 from os import path as osp
 if osp.isfile(osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), ".portage_not_installed")):
 	sys.path.insert(0, osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), "pym"))
 import portage
 portage._internal_caller = True
 from portage import os
-from portage import _unicode_decode
-from portage.dispatch_conf import diffstatusoutput
+from portage import _encodings, _unicode_decode
+from portage.dispatch_conf import diffstatusoutput, diff_mixed
 from portage.process import find_binary, spawn
 
 FIND_EXTANT_CONFIGS  = "find '%s' %s -name '._cfg????_%s' ! -name '.*~' ! -iname '.*.bak' -print"
@@ -72,6 +72,11 @@ def cmd_var_is_valid(cmd):
 
     return find_binary(cmd[0]) is not None
 
+def diff(file1, file2):
+    return diff_mixed(
+        functools.partial(diffstatusoutput, DIFF_CONTENTS),
+        file1, file2)
+
 class dispatch:
     options = {}
 
@@ -89,8 +94,6 @@ class dispatch:
                or not os.path.exists(self.options["log-file"]):
                 open(self.options["log-file"], 'w').close() # Truncate it
                 os.chmod(self.options["log-file"], 0o600)
-        else:
-            self.options["log-file"] = "/dev/null"
 
         pager = self.options.get("pager")
         if pager is None or not cmd_var_is_valid(pager):
@@ -148,9 +151,6 @@ class dispatch:
             portage.util.shlex_split(
             portage.settings.get('CONFIG_PROTECT_MASK', '')))
 
-        def diff(file1, file2):
-            return diffstatusoutput(DIFF_CONTENTS, file1, file2)
-
         #
         # Remove new configs identical to current
         #                  and
@@ -166,7 +166,7 @@ class dispatch:
                 mrgfail = portage.dispatch_conf.rcs_archive(archive, conf['current'], conf['new'], mrgconf)
             else:
                 mrgfail = portage.dispatch_conf.file_archive(archive, conf['current'], conf['new'], mrgconf)
-            if os.path.exists(archive + '.dist'):
+            if os.path.lexists(archive + '.dist'):
                 unmodified = len(diff(conf['current'], archive + '.dist')[1]) == 0
             else:
                 unmodified = 0
@@ -181,7 +181,7 @@ class dispatch:
 
             if newconf == mrgconf and \
                 self.options.get('ignore-previously-merged') != 'yes' and \
-                os.path.exists(archive+'.dist') and \
+                os.path.lexists(archive+'.dist') and \
                 len(diff(archive+'.dist', conf['new'])[1]) == 0:
                 # The current update is identical to the archived .dist
                 # version that has previously been merged.
@@ -254,6 +254,11 @@ class dispatch:
 
         valid_input = "qhtnmlezu"
 
+        def diff_pager(file1, file2):
+            cmd = self.options['diff'] % (file1, file2)
+            cmd += pager
+            spawn_shell(cmd)
+
         for conf in confs:
             count = count + 1
 
@@ -266,14 +271,10 @@ class dispatch:
             while 1:
                 clear_screen()
                 if show_new_diff:
-                    cmd = self.options['diff'] % (conf['new'], mrgconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_mixed(diff_pager, conf['new'], mrgconf)
                     show_new_diff = 0
                 else:
-                    cmd = self.options['diff'] % (conf['current'], newconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_mixed(diff_pager, conf['current'], newconf)
 
                 print()
                 print('>> (%i of %i) -- %s' % (count, len(confs), conf ['current']))
@@ -357,7 +358,12 @@ class dispatch:
     def replace (self, newconf, curconf):
         """Replace current config with the new/merged version.  Also logs
         the diff of what changed into the configured log file."""
-        os.system((DIFF_CONTENTS % (curconf, newconf)) + '>>' + self.options["log-file"])
+        if "log-file" in self.options:
+            status, output = diff(curconf, newconf)
+            with io.open(self.options["log-file"], mode="a",
+                encoding=_encodings["stdio"]) as f:
+                f.write(output + "\n")
+
         try:
             os.rename(newconf, curconf)
         except (IOError, os.error) as why:
diff --git a/pym/portage/dispatch_conf.py b/pym/portage/dispatch_conf.py
index 113d965..7fb99d0 100644
--- a/pym/portage/dispatch_conf.py
+++ b/pym/portage/dispatch_conf.py
@@ -6,11 +6,12 @@
 # Library by Wayne Davison <gentoo@blorf.net>, derived from code
 # written by Jeremy Wohl (http://igmus.org)
 
-from __future__ import print_function
+from __future__ import print_function, unicode_literals
 
-import os, shutil, subprocess, sys
+import functools, io, os, shutil, stat, subprocess, sys, tempfile
 
 import portage
+from portage import _encodings
 from portage.env.loaders import KeyValuePairFileLoader
 from portage.localization import _
 from portage.util import shlex_split, varexpand
@@ -50,6 +51,53 @@ def diffstatusoutput(cmd, file1, file2):
 		output = output[:-1]
 	return (proc.wait(), output)
 
+def diff_mixed(func, file1, file2):
+	tempdir = None
+	try:
+		if os.path.islink(file1) and \
+			not os.path.islink(file2) and \
+			os.path.isfile(file1) and \
+			os.path.isfile(file2):
+			# If a regular file replaces a symlink to a regular
+			# file, then show the diff between the regular files
+			# (bug #330221).
+			diff_files = (file2, file2)
+		else:
+			files = [file1, file2]
+			diff_files = [file1, file2]
+			for i in range(len(diff_files)):
+				st = os.lstat(diff_files[i])
+				if stat.S_ISREG(st.st_mode):
+					continue
+
+				if tempdir is None:
+					tempdir = tempfile.mkdtemp()
+				diff_files[i] = os.path.join(tempdir, "%d" % i)
+				if stat.S_ISLNK(st.st_mode):
+					link_dest = os.readlink(files[i])
+					content = "SYM: %s -> %s\n" % \
+						(file1, link_dest)
+				elif stat.S_ISDIR(st.st_mode):
+					content = "DIR: %s\n" % (file1,)
+				elif stat.S_ISFIFO(st.st_mode):
+					content = "FIF: %s\n" % (file1,)
+				else:
+					content = "DEV: %s\n" % (file1,)
+				with io.open(diff_files[i], mode='w',
+					encoding=_encodings['stdio']) as f:
+					f.write(content)
+
+		return func(diff_files[0], diff_files[1])
+
+	finally:
+		if tempdir is not None:
+			shutil.rmtree(tempdir)
+
+def diffstatusoutput_symlink(cmd, file1, file2):
+	return diff_mixed(
+		functools.partial(diffstatusoutput, cmd),
+		file1, file2)
+
 def read_config(mandatory_opts):
 	eprefix = portage.settings["EPREFIX"]
 	if portage._not_installed:
@@ -103,35 +151,57 @@ def rcs_archive(archive, curconf, newconf, mrgconf):
 	except OSError:
 		pass
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if os.path.exists(archive + ',v'):
+	if os.path.lexists(archive + ',v'):
 		os.system(RCS_LOCK + ' ' + archive)
 	os.system(RCS_PUT + ' ' + archive)
 
 	ret = 0
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		os.system(RCS_GET + ' -r' + RCS_BRANCH + ' ' + archive)
-		has_branch = os.path.exists(archive)
+		has_branch = os.path.lexists(archive)
 		if has_branch:
 			os.rename(archive, archive + '.dist')
 
 		try:
-			shutil.copy2(newconf, archive)
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), archive)
+			else:
+				shutil.copy2(newconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
 		if has_branch:
-			if mrgconf != '':
+			if mrgconf and os.path.isfile(archive) and \
+				os.path.isfile(mrgconf):
 				# This puts the results of the merge into mrgconf.
 				ret = os.system(RCS_MERGE % (archive, mrgconf))
-				mystat = os.lstat(newconf)
 				os.chmod(mrgconf, mystat.st_mode)
 				os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 		os.rename(archive, archive + '.dist.new')
@@ -153,10 +223,11 @@ def file_archive(archive, curconf, newconf, mrgconf):
 		pass
 
 	# Archive the current config file if it isn't already saved
-	if (os.path.exists(archive) and
-		len(diffstatusoutput("diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
+	if (os.path.lexists(archive) and
+		len(diffstatusoutput_symlink(
+		"diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
 		suf = 1
-		while suf < 9 and os.path.exists(archive + '.' + str(suf)):
+		while suf < 9 and os.path.lexists(archive + '.' + str(suf)):
 			suf += 1
 
 		while suf > 1:
@@ -165,26 +236,50 @@ def file_archive(archive, curconf, newconf, mrgconf):
 
 		os.rename(archive, archive + '.1')
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		# Save off new config file in the archive dir with .dist.new suffix
+		newconf_archive = archive + '.dist.new'
 		try:
-			shutil.copy2(newconf, archive + '.dist.new')
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), newconf_archive)
+			else:
+				shutil.copy2(newconf, newconf_archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive + '.dist.new', "reason": str(why)}, file=sys.stderr)
 
 		ret = 0
-		if mrgconf != '' and os.path.exists(archive + '.dist'):
+		if mrgconf and os.path.isfile(curconf) and \
+			os.path.isfile(newconf) and \
+			os.path.isfile(archive + '.dist'):
 			# This puts the results of the merge into mrgconf.
 			ret = os.system(DIFF3_MERGE % (curconf, archive + '.dist', newconf, mrgconf))
-			mystat = os.lstat(newconf)
 			os.chmod(mrgconf, mystat.st_mode)
 			os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 
@@ -195,7 +290,7 @@ def rcs_archive_post_process(archive):
 	"""Check in the archive file with the .dist.new suffix on the branch
 	and remove the one with the .dist suffix."""
 	os.rename(archive + '.dist.new', archive)
-	if os.path.exists(archive + '.dist'):
+	if os.path.lexists(archive + '.dist'):
 		# Commit the last-distributed version onto the branch.
 		os.system(RCS_LOCK + RCS_BRANCH + ' ' + archive)
 		os.system(RCS_PUT + ' -r' + RCS_BRANCH + ' ' + archive)
@@ -207,5 +302,5 @@ def rcs_archive_post_process(archive):
 
 def file_archive_post_process(archive):
 	"""Rename the archive file with the .dist.new suffix to a .dist suffix"""
-	if os.path.exists(archive + '.dist.new'):
+	if os.path.lexists(archive + '.dist.new'):
 		os.rename(archive + '.dist.new', archive + '.dist')
-- 
2.0.4



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

* [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, bug #485598
  2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " zmedico
@ 2014-10-26 11:12 ` zmedico
  2014-10-27  8:08   ` Alexander Berntsen
  2014-10-27 22:57 ` [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for " Zac Medico
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: zmedico @ 2014-10-26 11:12 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

From: Zac Medico <zmedico@gentoo.org>

Users may not want some symlinks to get clobbered, so protect them
with CONFIG_PROTECT. Changes were required in the dblink.mergeme method
and the new_protect_filename function. The unit tests demonstrate
operation in many different scenarios. For example:

 * regular file replaces regular file
 * regular file replaces symlink
 * regular file replaces directory
 * symlink replaces symlink
 * symlink replaces regular file
 * symlink replaces directory
 * directory replaces regular file
 * directory replaces symlink

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
 pym/portage/dbapi/vartree.py                    | 255 ++++++++++++---------
 pym/portage/tests/emerge/test_config_protect.py | 292 ++++++++++++++++++++++++
 pym/portage/util/__init__.py                    |  35 ++-
 3 files changed, 463 insertions(+), 119 deletions(-)
 create mode 100644 pym/portage/tests/emerge/test_config_protect.py

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index e21135a..219ca16 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -4461,21 +4461,17 @@ class dblink(object):
 			# stat file once, test using S_* macros many times (faster that way)
 			mystat = os.lstat(mysrc)
 			mymode = mystat[stat.ST_MODE]
-			# handy variables; mydest is the target object on the live filesystems;
-			# mysrc is the source object in the temporary install dir
-			try:
-				mydstat = os.lstat(mydest)
-				mydmode = mydstat.st_mode
-			except OSError as e:
-				if e.errno != errno.ENOENT:
-					raise
-				del e
-				#dest file doesn't exist
-				mydstat = None
-				mydmode = None
+			mymd5 = None
+			myto = None
 
-			if stat.S_ISLNK(mymode):
-				# we are merging a symbolic link
+			if sys.hexversion >= 0x3030000:
+				mymtime = mystat.st_mtime_ns
+			else:
+				mymtime = mystat[stat.ST_MTIME]
+
+			if stat.S_ISREG(mymode):
+				mymd5 = perform_md5(mysrc, calc_prelink=calc_prelink)
+			elif stat.S_ISLNK(mymode):
 				# The file name of mysrc and the actual file that it points to
 				# will have earlier been forcefully converted to the 'merge'
 				# encoding if necessary, but the content of the symbolic link
@@ -4495,6 +4491,69 @@ class dblink(object):
 					os.unlink(mysrc)
 					os.symlink(myto, mysrc)
 
+				mymd5 = portage.checksum._new_md5(
+					_unicode_encode(myto)).hexdigest()
+
+			protected = False
+			if stat.S_ISLNK(mymode) or stat.S_ISREG(mymode):
+				protected = self.isprotected(mydest)
+
+				if stat.S_ISREG(mymode) and \
+					mystat.st_size == 0 and \
+					os.path.basename(mydest).startswith(".keep"):
+					protected = False
+
+			destmd5 = None
+			mydest_link = None
+			# handy variables; mydest is the target object on the live filesystems;
+			# mysrc is the source object in the temporary install dir
+			try:
+				mydstat = os.lstat(mydest)
+				mydmode = mydstat.st_mode
+				if protected:
+					if stat.S_ISLNK(mydmode):
+						# Read symlink target as bytes, in case the
+						# target path has a bad encoding.
+						mydest_link = _os.readlink(
+							_unicode_encode(mydest,
+							encoding=_encodings['merge'],
+							errors='strict'))
+						mydest_link = _unicode_decode(mydest_link,
+							encoding=_encodings['merge'],
+							errors='replace')
+
+						# For protection of symlinks, the md5
+						# of the link target path string is used
+						# for cfgfiledict (symlinks are
+						# protected since bug #485598).
+						destmd5 = portage.checksum._new_md5(
+							_unicode_encode(mydest_link)).hexdigest()
+
+					elif stat.S_ISREG(mydmode):
+						destmd5 = perform_md5(mydest,
+							calc_prelink=calc_prelink)
+			except (FileNotFound, OSError) as e:
+				if isinstance(e, OSError) and e.errno != errno.ENOENT:
+					raise
+				#dest file doesn't exist
+				mydstat = None
+				mydmode = None
+				mydest_link = None
+				destmd5 = None
+
+			moveme = True
+			if protected:
+				mydest, protected, moveme = self._protect(cfgfiledict,
+					protect_if_modified, mymd5, myto, mydest,
+					myrealdest, mydmode, destmd5, mydest_link)
+
+			zing = "!!!"
+			if not moveme:
+				# confmem rejected this update
+				zing = "---"
+
+			if stat.S_ISLNK(mymode):
+				# we are merging a symbolic link
 				# Pass in the symlink target in order to bypass the
 				# os.readlink() call inside abssymlink(), since that
 				# call is unsafe if the merge encoding is not ascii
@@ -4510,9 +4569,8 @@ class dblink(object):
 				# myrealto contains the path of the real file to which this symlink points.
 				# we can simply test for existence of this file to see if the target has been merged yet
 				myrealto = normalize_path(os.path.join(destroot, myabsto))
-				if mydmode!=None:
-					#destination exists
-					if stat.S_ISDIR(mydmode):
+				if mydmode is not None and stat.S_ISDIR(mydmode):
+					if not protected:
 						# we can't merge a symlink over a directory
 						newdest = self._new_backup_path(mydest)
 						msg = []
@@ -4525,22 +4583,6 @@ class dblink(object):
 						self._eerror("preinst", msg)
 						mydest = newdest
 
-					elif not stat.S_ISLNK(mydmode):
-						if os.path.exists(mysrc) and stat.S_ISDIR(os.stat(mysrc)[stat.ST_MODE]):
-							# Kill file blocking installation of symlink to dir #71787
-							pass
-						elif self.isprotected(mydest):
-							# Use md5 of the target in ${D} if it exists...
-							try:
-								newmd5 = perform_md5(join(srcroot, myabsto))
-							except FileNotFound:
-								# Maybe the target is merged already.
-								try:
-									newmd5 = perform_md5(myrealto)
-								except FileNotFound:
-									newmd5 = None
-							mydest = new_protect_filename(mydest, newmd5=newmd5)
-
 				# if secondhand is None it means we're operating in "force" mode and should not create a second hand.
 				if (secondhand != None) and (not os.path.exists(myrealto)):
 					# either the target directory doesn't exist yet or the target file doesn't exist -- or
@@ -4549,9 +4591,11 @@ class dblink(object):
 					secondhand.append(mysrc[len(srcroot):])
 					continue
 				# unlinking no longer necessary; "movefile" will overwrite symlinks atomically and correctly
-				mymtime = movefile(mysrc, mydest, newmtime=thismtime,
-					sstat=mystat, mysettings=self.settings,
-					encoding=_encodings['merge'])
+				if moveme:
+					zing = ">>>"
+					mymtime = movefile(mysrc, mydest, newmtime=thismtime,
+						sstat=mystat, mysettings=self.settings,
+						encoding=_encodings['merge'])
 
 				try:
 					self._merged_path(mydest, os.lstat(mydest))
@@ -4567,7 +4611,7 @@ class dblink(object):
 							[_("QA Notice: Symbolic link /%s points to /%s which does not exist.")
 							% (relative_path, myabsto)])
 
-					showMessage(">>> %s -> %s\n" % (mydest, myto))
+					showMessage("%s %s -> %s\n" % (zing, mydest, myto))
 					if sys.hexversion >= 0x3030000:
 						outfile.write("sym "+myrealdest+" -> "+myto+" "+str(mymtime // 1000000000)+"\n")
 					else:
@@ -4589,7 +4633,8 @@ class dblink(object):
 						if dflags != 0:
 							bsd_chflags.lchflags(mydest, 0)
 
-					if not os.access(mydest, os.W_OK):
+					if not stat.S_ISLNK(mydmode) and \
+						not os.access(mydest, os.W_OK):
 						pkgstuff = pkgsplit(self.pkg)
 						writemsg(_("\n!!! Cannot write to '%s'.\n") % mydest, noiselevel=-1)
 						writemsg(_("!!! Please check permissions and directories for broken symlinks.\n"))
@@ -4678,14 +4723,8 @@ class dblink(object):
 
 			elif stat.S_ISREG(mymode):
 				# we are merging a regular file
-				mymd5 = perform_md5(mysrc, calc_prelink=calc_prelink)
-				# calculate config file protection stuff
-				mydestdir = os.path.dirname(mydest)
-				moveme = 1
-				zing = "!!!"
-				mymtime = None
-				protected = self.isprotected(mydest)
-				if mydmode is not None and stat.S_ISDIR(mydmode):
+				if not protected and \
+					mydmode is not None and stat.S_ISDIR(mydmode):
 						# install of destination is blocked by an existing directory with the same name
 						newdest = self._new_backup_path(mydest)
 						msg = []
@@ -4698,73 +4737,6 @@ class dblink(object):
 						self._eerror("preinst", msg)
 						mydest = newdest
 
-				elif mydmode is None or stat.S_ISREG(mydmode) or \
-					(stat.S_ISLNK(mydmode) and os.path.exists(mydest)
-					and stat.S_ISREG(os.stat(mydest)[stat.ST_MODE])):
-						# install of destination is blocked by an existing regular file,
-						# or by a symlink to an existing regular file;
-						# now, config file management may come into play.
-						# we only need to tweak mydest if cfg file management is in play.
-						destmd5 = None
-						if protected and mydmode is not None:
-							destmd5 = perform_md5(mydest, calc_prelink=calc_prelink)
-							if protect_if_modified:
-								contents_key = \
-									self._installed_instance._match_contents(myrealdest)
-								if contents_key:
-									inst_info = self._installed_instance.getcontents()[contents_key]
-									if inst_info[0] == "obj" and inst_info[2] == destmd5:
-										protected = False
-
-						if protected:
-							# we have a protection path; enable config file management.
-							cfgprot = 0
-							cfgprot_force = False
-							if mydmode is None:
-								if self._installed_instance is not None and \
-									self._installed_instance._match_contents(
-									myrealdest) is not False:
-									# If the file doesn't exist, then it may
-									# have been deleted or renamed by the
-									# admin. Therefore, force the file to be
-									# merged with a ._cfg name, so that the
-									# admin will be prompted for this update
-									# (see bug #523684).
-									cfgprot_force = True
-									moveme = True
-									cfgprot = True
-							elif mymd5 == destmd5:
-								#file already in place; simply update mtimes of destination
-								moveme = 1
-							else:
-								if mymd5 == cfgfiledict.get(myrealdest, [None])[0]:
-									""" An identical update has previously been
-									merged.  Skip it unless the user has chosen
-									--noconfmem."""
-									moveme = cfgfiledict["IGNORE"]
-									cfgprot = cfgfiledict["IGNORE"]
-									if not moveme:
-										zing = "---"
-										if sys.hexversion >= 0x3030000:
-											mymtime = mystat.st_mtime_ns
-										else:
-											mymtime = mystat[stat.ST_MTIME]
-								else:
-									moveme = 1
-									cfgprot = 1
-							if moveme:
-								# Merging a new file, so update confmem.
-								cfgfiledict[myrealdest] = [mymd5]
-							elif destmd5 == cfgfiledict.get(myrealdest, [None])[0]:
-								"""A previously remembered update has been
-								accepted, so it is removed from confmem."""
-								del cfgfiledict[myrealdest]
-
-							if cfgprot:
-								mydest = new_protect_filename(mydest,
-									newmd5=mymd5,
-									force=cfgprot_force)
-
 				# whether config protection or not, we merge the new file the
 				# same way.  Unless moveme=0 (blocking directory)
 				if moveme:
@@ -4820,6 +4792,63 @@ class dblink(object):
 					outfile.write("dev %s\n" % myrealdest)
 				showMessage(zing + " " + mydest + "\n")
 
+	def _protect(self, cfgfiledict, protect_if_modified, mymd5, myto,
+		mydest, myrealdest, mydmode, destmd5, mydest_link):
+
+		moveme = True
+		protected = True
+		force = False
+		k = False
+		if self._installed_instance is not None:
+			k = self._installed_instance._match_contents(myrealdest)
+		if k is not False:
+			if mydmode is None:
+				# If the file doesn't exist, then it may
+				# have been deleted or renamed by the
+				# admin. Therefore, force the file to be
+				# merged with a ._cfg name, so that the
+				# admin will be prompted for this update
+				# (see bug #523684).
+				force = True
+
+			elif protect_if_modified:
+				data = self._installed_instance.getcontents()[k]
+				if data[0] == "obj" and data[2] == destmd5:
+					protected = False
+				elif data[0] == "sym" and data[2] == mydest_link:
+					protected = False
+
+		if protected and mydmode is not None:
+			# we have a protection path; enable config file management.
+			if mymd5 != destmd5 and \
+				mymd5 == cfgfiledict.get(myrealdest, [None])[0]:
+				# An identical update has previously been
+				# merged.  Skip it unless the user has chosen
+				# --noconfmem.
+				moveme = protected = bool(cfgfiledict["IGNORE"])
+
+			if protected and \
+				(mydest_link is not None or myto is not None) and \
+				mydest_link != myto:
+				# If either one is a symlink, and they are not
+				# identical symlinks, then force config protection.
+				force = True
+
+			if moveme:
+				# Merging a new file, so update confmem.
+				cfgfiledict[myrealdest] = [mymd5]
+			elif destmd5 == cfgfiledict.get(myrealdest, [None])[0]:
+				# A previously remembered update has been
+				# accepted, so it is removed from confmem.
+				del cfgfiledict[myrealdest]
+
+		if protected and moveme:
+			mydest = new_protect_filename(mydest,
+				newmd5=(mydest_link or mymd5),
+				force=force)
+
+		return mydest, protected, moveme
+
 	def _merged_path(self, path, lstatobj, exists=True):
 		previous_path = self._device_path_map.get(lstatobj.st_dev)
 		if previous_path is None or previous_path is False or \
diff --git a/pym/portage/tests/emerge/test_config_protect.py b/pym/portage/tests/emerge/test_config_protect.py
new file mode 100644
index 0000000..5d7d8e9
--- /dev/null
+++ b/pym/portage/tests/emerge/test_config_protect.py
@@ -0,0 +1,292 @@
+# Copyright 2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from __future__ import unicode_literals
+
+import io
+from functools import partial
+import shutil
+import stat
+import subprocess
+import sys
+import time
+
+import portage
+from portage import os
+from portage import _encodings, _unicode_decode
+from portage.const import BASH_BINARY, PORTAGE_PYM_PATH
+from portage.process import find_binary
+from portage.tests import TestCase
+from portage.tests.resolver.ResolverPlayground import ResolverPlayground
+from portage.util import (ensure_dirs, find_updated_config_files,
+	shlex_split)
+
+class ConfigProtectTestCase(TestCase):
+
+	def testConfigProtect(self):
+		"""
+		Demonstrates many different scenarios. For example:
+
+		 * regular file replaces regular file
+		 * regular file replaces symlink
+		 * regular file replaces directory
+		 * symlink replaces symlink
+		 * symlink replaces regular file
+		 * symlink replaces directory
+		 * directory replaces regular file
+		 * directory replaces symlink
+		"""
+
+		debug = False
+
+		content_A_1 = """
+S="${WORKDIR}"
+
+src_install() {
+	insinto /etc/A
+	keepdir /etc/A/dir_a
+	keepdir /etc/A/symlink_replaces_dir
+	keepdir /etc/A/regular_replaces_dir
+	echo regular_a_1 > "${T}"/regular_a
+	doins "${T}"/regular_a
+	echo regular_b_1 > "${T}"/regular_b
+	doins "${T}"/regular_b
+	dosym regular_a /etc/A/regular_replaces_symlink
+	dosym regular_b /etc/A/symlink_replaces_symlink
+	echo regular_replaces_regular_1 > \
+		"${T}"/regular_replaces_regular
+	doins "${T}"/regular_replaces_regular
+	echo symlink_replaces_regular > \
+		"${T}"/symlink_replaces_regular
+	doins "${T}"/symlink_replaces_regular
+}
+
+"""
+
+		content_A_2 = """
+S="${WORKDIR}"
+
+src_install() {
+	insinto /etc/A
+	keepdir /etc/A/dir_a
+	dosym dir_a /etc/A/symlink_replaces_dir
+	echo regular_replaces_dir > "${T}"/regular_replaces_dir
+	doins "${T}"/regular_replaces_dir
+	echo regular_a_2 > "${T}"/regular_a
+	doins "${T}"/regular_a
+	echo regular_b_2 > "${T}"/regular_b
+	doins "${T}"/regular_b
+	echo regular_replaces_symlink > \
+		"${T}"/regular_replaces_symlink
+	doins "${T}"/regular_replaces_symlink
+	dosym regular_b /etc/A/symlink_replaces_symlink
+	echo regular_replaces_regular_2 > \
+		"${T}"/regular_replaces_regular
+	doins "${T}"/regular_replaces_regular
+	dosym regular_a /etc/A/symlink_replaces_regular
+}
+
+"""
+
+		ebuilds = {
+			"dev-libs/A-1": {
+				"EAPI" : "5",
+				"IUSE" : "+flag",
+				"KEYWORDS": "x86",
+				"LICENSE": "GPL-2",
+				"MISC_CONTENT": content_A_1,
+			},
+			"dev-libs/A-2": {
+				"EAPI" : "5",
+				"IUSE" : "+flag",
+				"KEYWORDS": "x86",
+				"LICENSE": "GPL-2",
+				"MISC_CONTENT": content_A_2,
+			},
+		}
+
+		playground = ResolverPlayground(
+			ebuilds=ebuilds, debug=debug)
+		settings = playground.settings
+		eprefix = settings["EPREFIX"]
+		eroot = settings["EROOT"]
+		var_cache_edb = os.path.join(eprefix, "var", "cache", "edb")
+
+		portage_python = portage._python_interpreter
+		dispatch_conf_cmd = (portage_python, "-b", "-Wd",
+			os.path.join(self.sbindir, "dispatch-conf"))
+		emerge_cmd = (portage_python, "-b", "-Wd",
+			os.path.join(self.bindir, "emerge"))
+		etc_update_cmd = (BASH_BINARY,
+			os.path.join(self.sbindir, "etc-update"))
+		etc_update_auto = etc_update_cmd + ("--automode", "-5",)
+
+		config_protect = "/etc"
+
+		def modify_files(dir_path):
+			for name in os.listdir(dir_path):
+				path = os.path.join(dir_path, name)
+				st = os.lstat(path)
+				if stat.S_ISREG(st.st_mode):
+					with io.open(path, mode='a',
+						encoding=_encodings["stdio"]) as f:
+						f.write("modified at %d\n" % time.time())
+				elif stat.S_ISLNK(st.st_mode):
+					old_dest = os.readlink(path)
+					os.unlink(path)
+					os.symlink(old_dest +
+						" modified at %d" % time.time(), path)
+
+		def updated_config_files(count):
+			self.assertEqual(count,
+				sum(len(x[1]) for x in find_updated_config_files(eroot,
+				shlex_split(config_protect))))
+
+		test_commands = (
+			etc_update_cmd,
+			dispatch_conf_cmd,
+			emerge_cmd + ("-1", "=dev-libs/A-1"),
+			partial(updated_config_files, 0),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 2),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 0),
+			# Test bug #523684, where a file renamed or removed by the
+			# admin forces replacement files to be merged with config
+			# protection.
+			partial(shutil.rmtree,
+				os.path.join(eprefix, "etc", "A")),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 8),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			# Modify some config files, and verify that it triggers
+			# config protection.
+			partial(modify_files,
+				os.path.join(eroot, "etc", "A")),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 6),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			# Modify some config files, downgrade to A-1, and verify
+			# that config protection works properly when the file
+			# types are changing.
+			partial(modify_files,
+				os.path.join(eroot, "etc", "A")),
+			emerge_cmd + ("-1", "--noconfmem", "=dev-libs/A-1"),
+			partial(updated_config_files, 6),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+		)
+
+		distdir = playground.distdir
+		fake_bin = os.path.join(eprefix, "bin")
+		portage_tmpdir = os.path.join(eprefix, "var", "tmp", "portage")
+
+		path =  os.environ.get("PATH")
+		if path is not None and not path.strip():
+			path = None
+		if path is None:
+			path = ""
+		else:
+			path = ":" + path
+		path = fake_bin + path
+
+		pythonpath =  os.environ.get("PYTHONPATH")
+		if pythonpath is not None and not pythonpath.strip():
+			pythonpath = None
+		if pythonpath is not None and \
+			pythonpath.split(":")[0] == PORTAGE_PYM_PATH:
+			pass
+		else:
+			if pythonpath is None:
+				pythonpath = ""
+			else:
+				pythonpath = ":" + pythonpath
+			pythonpath = PORTAGE_PYM_PATH + pythonpath
+
+		env = {
+			"PORTAGE_OVERRIDE_EPREFIX" : eprefix,
+			"CLEAN_DELAY" : "0",
+			"CONFIG_PROTECT": config_protect,
+			"DISTDIR" : distdir,
+			"EMERGE_DEFAULT_OPTS": "-v",
+			"EMERGE_WARNING_DELAY" : "0",
+			"INFODIR" : "",
+			"INFOPATH" : "",
+			"PATH" : path,
+			"PORTAGE_INST_GID" : str(portage.data.portage_gid),
+			"PORTAGE_INST_UID" : str(portage.data.portage_uid),
+			"PORTAGE_PYTHON" : portage_python,
+			"PORTAGE_REPOSITORIES" : settings.repositories.config_string(),
+			"PORTAGE_TMPDIR" : portage_tmpdir,
+			"PYTHONPATH" : pythonpath,
+			"__PORTAGE_TEST_PATH_OVERRIDE" : fake_bin,
+		}
+
+		if "__PORTAGE_TEST_HARDLINK_LOCKS" in os.environ:
+			env["__PORTAGE_TEST_HARDLINK_LOCKS"] = \
+				os.environ["__PORTAGE_TEST_HARDLINK_LOCKS"]
+
+		dirs = [distdir, fake_bin, portage_tmpdir,
+			var_cache_edb]
+		etc_symlinks = ("dispatch-conf.conf", "etc-update.conf")
+		# Override things that may be unavailable, or may have portability
+		# issues when running tests in exotic environments.
+		#   prepstrip - bug #447810 (bash read builtin EINTR problem)
+		true_symlinks = ["prepstrip", "scanelf"]
+		true_binary = find_binary("true")
+		self.assertEqual(true_binary is None, False,
+			"true command not found")
+		try:
+			for d in dirs:
+				ensure_dirs(d)
+			for x in true_symlinks:
+				os.symlink(true_binary, os.path.join(fake_bin, x))
+			for x in etc_symlinks:
+				os.symlink(os.path.join(self.cnf_etc_path, x),
+					os.path.join(eprefix, "etc", x))
+			with open(os.path.join(var_cache_edb, "counter"), 'wb') as f:
+				f.write(b"100")
+
+			if debug:
+				# The subprocess inherits both stdout and stderr, for
+				# debugging purposes.
+				stdout = None
+			else:
+				# The subprocess inherits stderr so that any warnings
+				# triggered by python -Wd will be visible.
+				stdout = subprocess.PIPE
+
+			for args in test_commands:
+
+				if hasattr(args, '__call__'):
+					args()
+					continue
+
+				if isinstance(args[0], dict):
+					local_env = env.copy()
+					local_env.update(args[0])
+					args = args[1:]
+				else:
+					local_env = env
+
+				proc = subprocess.Popen(args,
+					env=local_env, stdout=stdout)
+
+				if debug:
+					proc.wait()
+				else:
+					output = proc.stdout.readlines()
+					proc.wait()
+					proc.stdout.close()
+					if proc.returncode != os.EX_OK:
+						for line in output:
+							sys.stderr.write(_unicode_decode(line))
+
+				self.assertEqual(os.EX_OK, proc.returncode,
+					"emerge failed with args %s" % (args,))
+		finally:
+			playground.cleanup()
diff --git a/pym/portage/util/__init__.py b/pym/portage/util/__init__.py
index fe79942..ad3a351 100644
--- a/pym/portage/util/__init__.py
+++ b/pym/portage/util/__init__.py
@@ -1676,13 +1676,36 @@ def new_protect_filename(mydest, newmd5=None, force=False):
 	old_pfile = normalize_path(os.path.join(real_dirname, last_pfile))
 	if last_pfile and newmd5:
 		try:
-			last_pfile_md5 = portage.checksum._perform_md5_merge(old_pfile)
-		except FileNotFound:
-			# The file suddenly disappeared or it's a broken symlink.
-			pass
+			old_pfile_st = _os_merge.lstat(old_pfile)
+		except OSError as e:
+			if e.errno != errno.ENOENT:
+				raise
 		else:
-			if last_pfile_md5 == newmd5:
-				return old_pfile
+			if stat.S_ISLNK(old_pfile_st.st_mode):
+				try:
+					# Read symlink target as bytes, in case the
+					# target path has a bad encoding.
+					pfile_link = _os.readlink(_unicode_encode(old_pfile,
+						encoding=_encodings['merge'], errors='strict'))
+				except OSError:
+					if e.errno != errno.ENOENT:
+						raise
+				else:
+					pfile_link = _unicode_decode(
+						encoding=_encodings['merge'], errors='replace')
+					if pfile_link == newmd5:
+						return old_pfile
+			else:
+				try:
+					last_pfile_md5 = \
+						portage.checksum._perform_md5_merge(old_pfile)
+				except FileNotFound:
+					# The file suddenly disappeared or it's a
+					# broken symlink.
+					pass
+				else:
+					if last_pfile_md5 == newmd5:
+						return old_pfile
 	return new_pfile
 
 def find_updated_config_files(target_root, config_protect):
-- 
2.0.4



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

* Re: [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, bug #485598
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, " zmedico
@ 2014-10-27  8:08   ` Alexander Berntsen
  2014-10-27  9:07     ` Zac Medico
  2014-10-27 20:35     ` Zac Medico
  0 siblings, 2 replies; 10+ messages in thread
From: Alexander Berntsen @ 2014-10-27  8:08 UTC (permalink / raw
  To: gentoo-portage-dev

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

I'm not sure how anyone is seriously supposed to do an in-depth review
of 500+ changes. For what it's worth, what I could stomach reading of
the vartree changes, looked relatively sane. The commit messages also
seem to indicate you're doing something sane.

Is there no way of splitting these things into smaller commits that
make sense? As they are right now, it's almost pointless to even look
at the diff. One would have to look at the old and new source in their
entireties next to each other to get the bigger picture. (I might find
time for this later this week.)
- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iF4EAREIAAYFAlRN/WEACgkQRtClrXBQc7VEhwD9Gwa24KWNL47fjpcs375y7UEX
aAtuebxbhZSm/fq5xpAA/jDfmH+/rmJGARDr4ds8UyVtCkKfvTnD1ClGa9biRuI5
=8rjH
-----END PGP SIGNATURE-----


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

* Re: [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, bug #485598
  2014-10-27  8:08   ` Alexander Berntsen
@ 2014-10-27  9:07     ` Zac Medico
  2014-10-27 20:35     ` Zac Medico
  1 sibling, 0 replies; 10+ messages in thread
From: Zac Medico @ 2014-10-27  9:07 UTC (permalink / raw
  To: gentoo-portage-dev

On 10/27/2014 01:08 AM, Alexander Berntsen wrote:
> I'm not sure how anyone is seriously supposed to do an in-depth review
> of 500+ changes. For what it's worth, what I could stomach reading of
> the vartree changes, looked relatively sane. The commit messages also
> seem to indicate you're doing something sane.
> 
> Is there no way of splitting these things into smaller commits that
> make sense?

I think the current split into 3 patches is as small as we should go
here. We've got 3 logical parts, and it makes sense to look at all the
changes together for each of those parts.

> As they are right now, it's almost pointless to even look
> at the diff. One would have to look at the old and new source in their
> entireties next to each other to get the bigger picture. (I might find
> time for this later this week.)

The dispatch-conf and etc-update changes should be easier to review
without needing more context than what's in the patches. All of the
changes there simply to take code that only supported regular files and
fix it to handle symlinks differently from regular files.

The bigger picture is harder to see in the vartree patch, since
dblink.mergeme is such a large method. So, it may help if I give some
more overview in the commit message. The changes to dblink.mergeme do 3
things:

1) Move the bulk of config protection logic from dblink.mergeme to a new
dblink._protect method. The new method only returns 3 variables, which
makes it easier to understand how config protection interacts with the
dblink.mergeme code that uses those variables. This is important, since
dblink.mergeme has so many variables.

2) Initialize more variables at the beginning of dblink.mergeme, since
those variables are used by the dblink._protect method.

3) Use the variables returned from dblink._protect to trigger
appropriate behavior later in dblink.mergeme.

The new_protect_filename changes are required since this function
compares the new file to old ._cfg* files that may already exist, in
order to avoid creating duplicate .cfg* files. In these comparisons, it
needs to handle symlinks differently from regular files.
-- 
Thanks,
Zac


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

* [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, bug #485598
  2014-10-27  8:08   ` Alexander Berntsen
  2014-10-27  9:07     ` Zac Medico
@ 2014-10-27 20:35     ` Zac Medico
  2014-11-03  3:54       ` Brian Dolbec
  1 sibling, 1 reply; 10+ messages in thread
From: Zac Medico @ 2014-10-27 20:35 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

Users may not want some symlinks to get clobbered, so protect them
with CONFIG_PROTECT. Changes were required in the dblink.mergeme method
and the new_protect_filename function.

The changes to dblink.mergeme do 3 things:

 * Move the bulk of config protection logic from dblink.mergeme to a
   new dblink._protect method. The new method only returns 3 variables,
   which makes it easier to understand how config protection interacts
   with the dblink.mergeme code that uses those variables. This is
   important, since dblink.mergeme has so many variables.

 * Initialize more variables at the beginning of dblink.mergeme, since
   those variables are used by the dblink._protect method.

 * Use the variables returned from dblink._protect to trigger
   appropriate behavior later in dblink.mergeme.

The new_protect_filename changes are required since this function
compares the new file to old ._cfg* files that may already exist, in
order to avoid creating duplicate ._cfg* files. In these comparisons,
it needs to handle symlinks differently from regular files.

The unit tests demonstrate operation in many different scenarios,
including:

 * regular file replaces regular file
 * regular file replaces symlink
 * regular file replaces directory
 * symlink replaces symlink
 * symlink replaces regular file
 * symlink replaces directory
 * directory replaces regular file
 * directory replaces symlink

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
This updated patch only adds to the commit message in order to provide some
information that may be helpful to reviewers of the patch. There are no
changes to the code.

 pym/portage/dbapi/vartree.py                    | 255 ++++++++++++---------
 pym/portage/tests/emerge/test_config_protect.py | 292 ++++++++++++++++++++++++
 pym/portage/util/__init__.py                    |  35 ++-
 3 files changed, 463 insertions(+), 119 deletions(-)
 create mode 100644 pym/portage/tests/emerge/test_config_protect.py

diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index e21135a..219ca16 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -4461,21 +4461,17 @@ class dblink(object):
 			# stat file once, test using S_* macros many times (faster that way)
 			mystat = os.lstat(mysrc)
 			mymode = mystat[stat.ST_MODE]
-			# handy variables; mydest is the target object on the live filesystems;
-			# mysrc is the source object in the temporary install dir
-			try:
-				mydstat = os.lstat(mydest)
-				mydmode = mydstat.st_mode
-			except OSError as e:
-				if e.errno != errno.ENOENT:
-					raise
-				del e
-				#dest file doesn't exist
-				mydstat = None
-				mydmode = None
+			mymd5 = None
+			myto = None
 
-			if stat.S_ISLNK(mymode):
-				# we are merging a symbolic link
+			if sys.hexversion >= 0x3030000:
+				mymtime = mystat.st_mtime_ns
+			else:
+				mymtime = mystat[stat.ST_MTIME]
+
+			if stat.S_ISREG(mymode):
+				mymd5 = perform_md5(mysrc, calc_prelink=calc_prelink)
+			elif stat.S_ISLNK(mymode):
 				# The file name of mysrc and the actual file that it points to
 				# will have earlier been forcefully converted to the 'merge'
 				# encoding if necessary, but the content of the symbolic link
@@ -4495,6 +4491,69 @@ class dblink(object):
 					os.unlink(mysrc)
 					os.symlink(myto, mysrc)
 
+				mymd5 = portage.checksum._new_md5(
+					_unicode_encode(myto)).hexdigest()
+
+			protected = False
+			if stat.S_ISLNK(mymode) or stat.S_ISREG(mymode):
+				protected = self.isprotected(mydest)
+
+				if stat.S_ISREG(mymode) and \
+					mystat.st_size == 0 and \
+					os.path.basename(mydest).startswith(".keep"):
+					protected = False
+
+			destmd5 = None
+			mydest_link = None
+			# handy variables; mydest is the target object on the live filesystems;
+			# mysrc is the source object in the temporary install dir
+			try:
+				mydstat = os.lstat(mydest)
+				mydmode = mydstat.st_mode
+				if protected:
+					if stat.S_ISLNK(mydmode):
+						# Read symlink target as bytes, in case the
+						# target path has a bad encoding.
+						mydest_link = _os.readlink(
+							_unicode_encode(mydest,
+							encoding=_encodings['merge'],
+							errors='strict'))
+						mydest_link = _unicode_decode(mydest_link,
+							encoding=_encodings['merge'],
+							errors='replace')
+
+						# For protection of symlinks, the md5
+						# of the link target path string is used
+						# for cfgfiledict (symlinks are
+						# protected since bug #485598).
+						destmd5 = portage.checksum._new_md5(
+							_unicode_encode(mydest_link)).hexdigest()
+
+					elif stat.S_ISREG(mydmode):
+						destmd5 = perform_md5(mydest,
+							calc_prelink=calc_prelink)
+			except (FileNotFound, OSError) as e:
+				if isinstance(e, OSError) and e.errno != errno.ENOENT:
+					raise
+				#dest file doesn't exist
+				mydstat = None
+				mydmode = None
+				mydest_link = None
+				destmd5 = None
+
+			moveme = True
+			if protected:
+				mydest, protected, moveme = self._protect(cfgfiledict,
+					protect_if_modified, mymd5, myto, mydest,
+					myrealdest, mydmode, destmd5, mydest_link)
+
+			zing = "!!!"
+			if not moveme:
+				# confmem rejected this update
+				zing = "---"
+
+			if stat.S_ISLNK(mymode):
+				# we are merging a symbolic link
 				# Pass in the symlink target in order to bypass the
 				# os.readlink() call inside abssymlink(), since that
 				# call is unsafe if the merge encoding is not ascii
@@ -4510,9 +4569,8 @@ class dblink(object):
 				# myrealto contains the path of the real file to which this symlink points.
 				# we can simply test for existence of this file to see if the target has been merged yet
 				myrealto = normalize_path(os.path.join(destroot, myabsto))
-				if mydmode!=None:
-					#destination exists
-					if stat.S_ISDIR(mydmode):
+				if mydmode is not None and stat.S_ISDIR(mydmode):
+					if not protected:
 						# we can't merge a symlink over a directory
 						newdest = self._new_backup_path(mydest)
 						msg = []
@@ -4525,22 +4583,6 @@ class dblink(object):
 						self._eerror("preinst", msg)
 						mydest = newdest
 
-					elif not stat.S_ISLNK(mydmode):
-						if os.path.exists(mysrc) and stat.S_ISDIR(os.stat(mysrc)[stat.ST_MODE]):
-							# Kill file blocking installation of symlink to dir #71787
-							pass
-						elif self.isprotected(mydest):
-							# Use md5 of the target in ${D} if it exists...
-							try:
-								newmd5 = perform_md5(join(srcroot, myabsto))
-							except FileNotFound:
-								# Maybe the target is merged already.
-								try:
-									newmd5 = perform_md5(myrealto)
-								except FileNotFound:
-									newmd5 = None
-							mydest = new_protect_filename(mydest, newmd5=newmd5)
-
 				# if secondhand is None it means we're operating in "force" mode and should not create a second hand.
 				if (secondhand != None) and (not os.path.exists(myrealto)):
 					# either the target directory doesn't exist yet or the target file doesn't exist -- or
@@ -4549,9 +4591,11 @@ class dblink(object):
 					secondhand.append(mysrc[len(srcroot):])
 					continue
 				# unlinking no longer necessary; "movefile" will overwrite symlinks atomically and correctly
-				mymtime = movefile(mysrc, mydest, newmtime=thismtime,
-					sstat=mystat, mysettings=self.settings,
-					encoding=_encodings['merge'])
+				if moveme:
+					zing = ">>>"
+					mymtime = movefile(mysrc, mydest, newmtime=thismtime,
+						sstat=mystat, mysettings=self.settings,
+						encoding=_encodings['merge'])
 
 				try:
 					self._merged_path(mydest, os.lstat(mydest))
@@ -4567,7 +4611,7 @@ class dblink(object):
 							[_("QA Notice: Symbolic link /%s points to /%s which does not exist.")
 							% (relative_path, myabsto)])
 
-					showMessage(">>> %s -> %s\n" % (mydest, myto))
+					showMessage("%s %s -> %s\n" % (zing, mydest, myto))
 					if sys.hexversion >= 0x3030000:
 						outfile.write("sym "+myrealdest+" -> "+myto+" "+str(mymtime // 1000000000)+"\n")
 					else:
@@ -4589,7 +4633,8 @@ class dblink(object):
 						if dflags != 0:
 							bsd_chflags.lchflags(mydest, 0)
 
-					if not os.access(mydest, os.W_OK):
+					if not stat.S_ISLNK(mydmode) and \
+						not os.access(mydest, os.W_OK):
 						pkgstuff = pkgsplit(self.pkg)
 						writemsg(_("\n!!! Cannot write to '%s'.\n") % mydest, noiselevel=-1)
 						writemsg(_("!!! Please check permissions and directories for broken symlinks.\n"))
@@ -4678,14 +4723,8 @@ class dblink(object):
 
 			elif stat.S_ISREG(mymode):
 				# we are merging a regular file
-				mymd5 = perform_md5(mysrc, calc_prelink=calc_prelink)
-				# calculate config file protection stuff
-				mydestdir = os.path.dirname(mydest)
-				moveme = 1
-				zing = "!!!"
-				mymtime = None
-				protected = self.isprotected(mydest)
-				if mydmode is not None and stat.S_ISDIR(mydmode):
+				if not protected and \
+					mydmode is not None and stat.S_ISDIR(mydmode):
 						# install of destination is blocked by an existing directory with the same name
 						newdest = self._new_backup_path(mydest)
 						msg = []
@@ -4698,73 +4737,6 @@ class dblink(object):
 						self._eerror("preinst", msg)
 						mydest = newdest
 
-				elif mydmode is None or stat.S_ISREG(mydmode) or \
-					(stat.S_ISLNK(mydmode) and os.path.exists(mydest)
-					and stat.S_ISREG(os.stat(mydest)[stat.ST_MODE])):
-						# install of destination is blocked by an existing regular file,
-						# or by a symlink to an existing regular file;
-						# now, config file management may come into play.
-						# we only need to tweak mydest if cfg file management is in play.
-						destmd5 = None
-						if protected and mydmode is not None:
-							destmd5 = perform_md5(mydest, calc_prelink=calc_prelink)
-							if protect_if_modified:
-								contents_key = \
-									self._installed_instance._match_contents(myrealdest)
-								if contents_key:
-									inst_info = self._installed_instance.getcontents()[contents_key]
-									if inst_info[0] == "obj" and inst_info[2] == destmd5:
-										protected = False
-
-						if protected:
-							# we have a protection path; enable config file management.
-							cfgprot = 0
-							cfgprot_force = False
-							if mydmode is None:
-								if self._installed_instance is not None and \
-									self._installed_instance._match_contents(
-									myrealdest) is not False:
-									# If the file doesn't exist, then it may
-									# have been deleted or renamed by the
-									# admin. Therefore, force the file to be
-									# merged with a ._cfg name, so that the
-									# admin will be prompted for this update
-									# (see bug #523684).
-									cfgprot_force = True
-									moveme = True
-									cfgprot = True
-							elif mymd5 == destmd5:
-								#file already in place; simply update mtimes of destination
-								moveme = 1
-							else:
-								if mymd5 == cfgfiledict.get(myrealdest, [None])[0]:
-									""" An identical update has previously been
-									merged.  Skip it unless the user has chosen
-									--noconfmem."""
-									moveme = cfgfiledict["IGNORE"]
-									cfgprot = cfgfiledict["IGNORE"]
-									if not moveme:
-										zing = "---"
-										if sys.hexversion >= 0x3030000:
-											mymtime = mystat.st_mtime_ns
-										else:
-											mymtime = mystat[stat.ST_MTIME]
-								else:
-									moveme = 1
-									cfgprot = 1
-							if moveme:
-								# Merging a new file, so update confmem.
-								cfgfiledict[myrealdest] = [mymd5]
-							elif destmd5 == cfgfiledict.get(myrealdest, [None])[0]:
-								"""A previously remembered update has been
-								accepted, so it is removed from confmem."""
-								del cfgfiledict[myrealdest]
-
-							if cfgprot:
-								mydest = new_protect_filename(mydest,
-									newmd5=mymd5,
-									force=cfgprot_force)
-
 				# whether config protection or not, we merge the new file the
 				# same way.  Unless moveme=0 (blocking directory)
 				if moveme:
@@ -4820,6 +4792,63 @@ class dblink(object):
 					outfile.write("dev %s\n" % myrealdest)
 				showMessage(zing + " " + mydest + "\n")
 
+	def _protect(self, cfgfiledict, protect_if_modified, mymd5, myto,
+		mydest, myrealdest, mydmode, destmd5, mydest_link):
+
+		moveme = True
+		protected = True
+		force = False
+		k = False
+		if self._installed_instance is not None:
+			k = self._installed_instance._match_contents(myrealdest)
+		if k is not False:
+			if mydmode is None:
+				# If the file doesn't exist, then it may
+				# have been deleted or renamed by the
+				# admin. Therefore, force the file to be
+				# merged with a ._cfg name, so that the
+				# admin will be prompted for this update
+				# (see bug #523684).
+				force = True
+
+			elif protect_if_modified:
+				data = self._installed_instance.getcontents()[k]
+				if data[0] == "obj" and data[2] == destmd5:
+					protected = False
+				elif data[0] == "sym" and data[2] == mydest_link:
+					protected = False
+
+		if protected and mydmode is not None:
+			# we have a protection path; enable config file management.
+			if mymd5 != destmd5 and \
+				mymd5 == cfgfiledict.get(myrealdest, [None])[0]:
+				# An identical update has previously been
+				# merged.  Skip it unless the user has chosen
+				# --noconfmem.
+				moveme = protected = bool(cfgfiledict["IGNORE"])
+
+			if protected and \
+				(mydest_link is not None or myto is not None) and \
+				mydest_link != myto:
+				# If either one is a symlink, and they are not
+				# identical symlinks, then force config protection.
+				force = True
+
+			if moveme:
+				# Merging a new file, so update confmem.
+				cfgfiledict[myrealdest] = [mymd5]
+			elif destmd5 == cfgfiledict.get(myrealdest, [None])[0]:
+				# A previously remembered update has been
+				# accepted, so it is removed from confmem.
+				del cfgfiledict[myrealdest]
+
+		if protected and moveme:
+			mydest = new_protect_filename(mydest,
+				newmd5=(mydest_link or mymd5),
+				force=force)
+
+		return mydest, protected, moveme
+
 	def _merged_path(self, path, lstatobj, exists=True):
 		previous_path = self._device_path_map.get(lstatobj.st_dev)
 		if previous_path is None or previous_path is False or \
diff --git a/pym/portage/tests/emerge/test_config_protect.py b/pym/portage/tests/emerge/test_config_protect.py
new file mode 100644
index 0000000..5d7d8e9
--- /dev/null
+++ b/pym/portage/tests/emerge/test_config_protect.py
@@ -0,0 +1,292 @@
+# Copyright 2014 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from __future__ import unicode_literals
+
+import io
+from functools import partial
+import shutil
+import stat
+import subprocess
+import sys
+import time
+
+import portage
+from portage import os
+from portage import _encodings, _unicode_decode
+from portage.const import BASH_BINARY, PORTAGE_PYM_PATH
+from portage.process import find_binary
+from portage.tests import TestCase
+from portage.tests.resolver.ResolverPlayground import ResolverPlayground
+from portage.util import (ensure_dirs, find_updated_config_files,
+	shlex_split)
+
+class ConfigProtectTestCase(TestCase):
+
+	def testConfigProtect(self):
+		"""
+		Demonstrates many different scenarios. For example:
+
+		 * regular file replaces regular file
+		 * regular file replaces symlink
+		 * regular file replaces directory
+		 * symlink replaces symlink
+		 * symlink replaces regular file
+		 * symlink replaces directory
+		 * directory replaces regular file
+		 * directory replaces symlink
+		"""
+
+		debug = False
+
+		content_A_1 = """
+S="${WORKDIR}"
+
+src_install() {
+	insinto /etc/A
+	keepdir /etc/A/dir_a
+	keepdir /etc/A/symlink_replaces_dir
+	keepdir /etc/A/regular_replaces_dir
+	echo regular_a_1 > "${T}"/regular_a
+	doins "${T}"/regular_a
+	echo regular_b_1 > "${T}"/regular_b
+	doins "${T}"/regular_b
+	dosym regular_a /etc/A/regular_replaces_symlink
+	dosym regular_b /etc/A/symlink_replaces_symlink
+	echo regular_replaces_regular_1 > \
+		"${T}"/regular_replaces_regular
+	doins "${T}"/regular_replaces_regular
+	echo symlink_replaces_regular > \
+		"${T}"/symlink_replaces_regular
+	doins "${T}"/symlink_replaces_regular
+}
+
+"""
+
+		content_A_2 = """
+S="${WORKDIR}"
+
+src_install() {
+	insinto /etc/A
+	keepdir /etc/A/dir_a
+	dosym dir_a /etc/A/symlink_replaces_dir
+	echo regular_replaces_dir > "${T}"/regular_replaces_dir
+	doins "${T}"/regular_replaces_dir
+	echo regular_a_2 > "${T}"/regular_a
+	doins "${T}"/regular_a
+	echo regular_b_2 > "${T}"/regular_b
+	doins "${T}"/regular_b
+	echo regular_replaces_symlink > \
+		"${T}"/regular_replaces_symlink
+	doins "${T}"/regular_replaces_symlink
+	dosym regular_b /etc/A/symlink_replaces_symlink
+	echo regular_replaces_regular_2 > \
+		"${T}"/regular_replaces_regular
+	doins "${T}"/regular_replaces_regular
+	dosym regular_a /etc/A/symlink_replaces_regular
+}
+
+"""
+
+		ebuilds = {
+			"dev-libs/A-1": {
+				"EAPI" : "5",
+				"IUSE" : "+flag",
+				"KEYWORDS": "x86",
+				"LICENSE": "GPL-2",
+				"MISC_CONTENT": content_A_1,
+			},
+			"dev-libs/A-2": {
+				"EAPI" : "5",
+				"IUSE" : "+flag",
+				"KEYWORDS": "x86",
+				"LICENSE": "GPL-2",
+				"MISC_CONTENT": content_A_2,
+			},
+		}
+
+		playground = ResolverPlayground(
+			ebuilds=ebuilds, debug=debug)
+		settings = playground.settings
+		eprefix = settings["EPREFIX"]
+		eroot = settings["EROOT"]
+		var_cache_edb = os.path.join(eprefix, "var", "cache", "edb")
+
+		portage_python = portage._python_interpreter
+		dispatch_conf_cmd = (portage_python, "-b", "-Wd",
+			os.path.join(self.sbindir, "dispatch-conf"))
+		emerge_cmd = (portage_python, "-b", "-Wd",
+			os.path.join(self.bindir, "emerge"))
+		etc_update_cmd = (BASH_BINARY,
+			os.path.join(self.sbindir, "etc-update"))
+		etc_update_auto = etc_update_cmd + ("--automode", "-5",)
+
+		config_protect = "/etc"
+
+		def modify_files(dir_path):
+			for name in os.listdir(dir_path):
+				path = os.path.join(dir_path, name)
+				st = os.lstat(path)
+				if stat.S_ISREG(st.st_mode):
+					with io.open(path, mode='a',
+						encoding=_encodings["stdio"]) as f:
+						f.write("modified at %d\n" % time.time())
+				elif stat.S_ISLNK(st.st_mode):
+					old_dest = os.readlink(path)
+					os.unlink(path)
+					os.symlink(old_dest +
+						" modified at %d" % time.time(), path)
+
+		def updated_config_files(count):
+			self.assertEqual(count,
+				sum(len(x[1]) for x in find_updated_config_files(eroot,
+				shlex_split(config_protect))))
+
+		test_commands = (
+			etc_update_cmd,
+			dispatch_conf_cmd,
+			emerge_cmd + ("-1", "=dev-libs/A-1"),
+			partial(updated_config_files, 0),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 2),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 0),
+			# Test bug #523684, where a file renamed or removed by the
+			# admin forces replacement files to be merged with config
+			# protection.
+			partial(shutil.rmtree,
+				os.path.join(eprefix, "etc", "A")),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 8),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			# Modify some config files, and verify that it triggers
+			# config protection.
+			partial(modify_files,
+				os.path.join(eroot, "etc", "A")),
+			emerge_cmd + ("-1", "=dev-libs/A-2"),
+			partial(updated_config_files, 6),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+			# Modify some config files, downgrade to A-1, and verify
+			# that config protection works properly when the file
+			# types are changing.
+			partial(modify_files,
+				os.path.join(eroot, "etc", "A")),
+			emerge_cmd + ("-1", "--noconfmem", "=dev-libs/A-1"),
+			partial(updated_config_files, 6),
+			etc_update_auto,
+			partial(updated_config_files, 0),
+		)
+
+		distdir = playground.distdir
+		fake_bin = os.path.join(eprefix, "bin")
+		portage_tmpdir = os.path.join(eprefix, "var", "tmp", "portage")
+
+		path =  os.environ.get("PATH")
+		if path is not None and not path.strip():
+			path = None
+		if path is None:
+			path = ""
+		else:
+			path = ":" + path
+		path = fake_bin + path
+
+		pythonpath =  os.environ.get("PYTHONPATH")
+		if pythonpath is not None and not pythonpath.strip():
+			pythonpath = None
+		if pythonpath is not None and \
+			pythonpath.split(":")[0] == PORTAGE_PYM_PATH:
+			pass
+		else:
+			if pythonpath is None:
+				pythonpath = ""
+			else:
+				pythonpath = ":" + pythonpath
+			pythonpath = PORTAGE_PYM_PATH + pythonpath
+
+		env = {
+			"PORTAGE_OVERRIDE_EPREFIX" : eprefix,
+			"CLEAN_DELAY" : "0",
+			"CONFIG_PROTECT": config_protect,
+			"DISTDIR" : distdir,
+			"EMERGE_DEFAULT_OPTS": "-v",
+			"EMERGE_WARNING_DELAY" : "0",
+			"INFODIR" : "",
+			"INFOPATH" : "",
+			"PATH" : path,
+			"PORTAGE_INST_GID" : str(portage.data.portage_gid),
+			"PORTAGE_INST_UID" : str(portage.data.portage_uid),
+			"PORTAGE_PYTHON" : portage_python,
+			"PORTAGE_REPOSITORIES" : settings.repositories.config_string(),
+			"PORTAGE_TMPDIR" : portage_tmpdir,
+			"PYTHONPATH" : pythonpath,
+			"__PORTAGE_TEST_PATH_OVERRIDE" : fake_bin,
+		}
+
+		if "__PORTAGE_TEST_HARDLINK_LOCKS" in os.environ:
+			env["__PORTAGE_TEST_HARDLINK_LOCKS"] = \
+				os.environ["__PORTAGE_TEST_HARDLINK_LOCKS"]
+
+		dirs = [distdir, fake_bin, portage_tmpdir,
+			var_cache_edb]
+		etc_symlinks = ("dispatch-conf.conf", "etc-update.conf")
+		# Override things that may be unavailable, or may have portability
+		# issues when running tests in exotic environments.
+		#   prepstrip - bug #447810 (bash read builtin EINTR problem)
+		true_symlinks = ["prepstrip", "scanelf"]
+		true_binary = find_binary("true")
+		self.assertEqual(true_binary is None, False,
+			"true command not found")
+		try:
+			for d in dirs:
+				ensure_dirs(d)
+			for x in true_symlinks:
+				os.symlink(true_binary, os.path.join(fake_bin, x))
+			for x in etc_symlinks:
+				os.symlink(os.path.join(self.cnf_etc_path, x),
+					os.path.join(eprefix, "etc", x))
+			with open(os.path.join(var_cache_edb, "counter"), 'wb') as f:
+				f.write(b"100")
+
+			if debug:
+				# The subprocess inherits both stdout and stderr, for
+				# debugging purposes.
+				stdout = None
+			else:
+				# The subprocess inherits stderr so that any warnings
+				# triggered by python -Wd will be visible.
+				stdout = subprocess.PIPE
+
+			for args in test_commands:
+
+				if hasattr(args, '__call__'):
+					args()
+					continue
+
+				if isinstance(args[0], dict):
+					local_env = env.copy()
+					local_env.update(args[0])
+					args = args[1:]
+				else:
+					local_env = env
+
+				proc = subprocess.Popen(args,
+					env=local_env, stdout=stdout)
+
+				if debug:
+					proc.wait()
+				else:
+					output = proc.stdout.readlines()
+					proc.wait()
+					proc.stdout.close()
+					if proc.returncode != os.EX_OK:
+						for line in output:
+							sys.stderr.write(_unicode_decode(line))
+
+				self.assertEqual(os.EX_OK, proc.returncode,
+					"emerge failed with args %s" % (args,))
+		finally:
+			playground.cleanup()
diff --git a/pym/portage/util/__init__.py b/pym/portage/util/__init__.py
index fe79942..ad3a351 100644
--- a/pym/portage/util/__init__.py
+++ b/pym/portage/util/__init__.py
@@ -1676,13 +1676,36 @@ def new_protect_filename(mydest, newmd5=None, force=False):
 	old_pfile = normalize_path(os.path.join(real_dirname, last_pfile))
 	if last_pfile and newmd5:
 		try:
-			last_pfile_md5 = portage.checksum._perform_md5_merge(old_pfile)
-		except FileNotFound:
-			# The file suddenly disappeared or it's a broken symlink.
-			pass
+			old_pfile_st = _os_merge.lstat(old_pfile)
+		except OSError as e:
+			if e.errno != errno.ENOENT:
+				raise
 		else:
-			if last_pfile_md5 == newmd5:
-				return old_pfile
+			if stat.S_ISLNK(old_pfile_st.st_mode):
+				try:
+					# Read symlink target as bytes, in case the
+					# target path has a bad encoding.
+					pfile_link = _os.readlink(_unicode_encode(old_pfile,
+						encoding=_encodings['merge'], errors='strict'))
+				except OSError:
+					if e.errno != errno.ENOENT:
+						raise
+				else:
+					pfile_link = _unicode_decode(
+						encoding=_encodings['merge'], errors='replace')
+					if pfile_link == newmd5:
+						return old_pfile
+			else:
+				try:
+					last_pfile_md5 = \
+						portage.checksum._perform_md5_merge(old_pfile)
+				except FileNotFound:
+					# The file suddenly disappeared or it's a
+					# broken symlink.
+					pass
+				else:
+					if last_pfile_md5 == newmd5:
+						return old_pfile
 	return new_pfile
 
 def find_updated_config_files(target_root, config_protect):
-- 
2.0.4



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

* [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598
  2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " zmedico
  2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, " zmedico
@ 2014-10-27 22:57 ` Zac Medico
  2014-10-27 23:04 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " Zac Medico
  2014-10-31 13:34 ` [gentoo-portage-dev] [PATCH] " Zac Medico
  4 siblings, 0 replies; 10+ messages in thread
From: Zac Medico @ 2014-10-27 22:57 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

This includes numerous logic adjustments that are needed to support
protected symlinks. The show_diff function now supports arbitrary
file types. For example, a diff between two symlinks looks like this:

-SYM: /foo/bar -> baz
+SYM: /foo/bar -> blah

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
This updated patch fixes show_diff to handle files that don't exist. These
are displayed as "/dev/null".

 bin/etc-update | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 87 insertions(+), 8 deletions(-)

diff --git a/bin/etc-update b/bin/etc-update
index 7ac6f0b..1d78e96 100755
--- a/bin/etc-update
+++ b/bin/etc-update
@@ -51,11 +51,15 @@ do_mv_ln() {
 	local src=${@:$(( $# - 1 )):1}
 	local dst=${@:$(( $# - 0 )):1}
 
-	if [[ -L ${dst} ]] ; then #330221
+	if [[ ! -L ${src} && -L ${dst} ]] ; then #330221
 		local lfile=$(readlink "${dst}")
 		[[ ${lfile} == /* ]] || lfile="${dst%/*}/${lfile}"
 		echo " Target is a symlink; replacing ${lfile}"
 		dst=${lfile}
+	elif [[ -d ${dst} && ! -L ${dst} ]] ; then
+		# If ${dst} is a directory, do not move the file
+		# inside of it if this fails.
+		rmdir "${dst}" || return
 	fi
 
 	mv "${opts[@]}" "${src}" "${dst}"
@@ -115,6 +119,24 @@ scan() {
 					continue 2
 				fi
 			done
+			if [[ -L ${file} ]] ; then
+				if [[ -L ${live_file} && \
+					$(readlink "${live_file}") == $(readlink "${file}") ]]
+				then
+					rm -f "${file}"
+					continue
+				fi
+				if [[ "${ofile:10}" != "${rfile:10}" ]] ||
+				   [[ ${opath} != ${rpath} ]]
+				then
+					: $(( ++count ))
+					echo "${live_file}" > "${TMP}"/files/${count}
+				fi
+				echo "${cfg_file}" >> "${TMP}"/files/${count}
+				ofile="${rfile}"
+				opath="${rpath}"
+				continue
+			fi
 			if [[ ! -f ${file} ]] ; then
 				${QUIET} || echo "Skipping non-file ${file} ..."
 				continue
@@ -124,7 +146,9 @@ scan() {
 			   [[ ${opath} != ${rpath} ]]
 			then
 				MATCHES=0
-				if [[ ${eu_automerge} == "yes" ]] ; then
+				if ! [[ -f ${cfg_file} && -f ${live_file} ]] ; then
+					MATCHES=0
+				elif [[ ${eu_automerge} == "yes" ]] ; then
 					if [[ ! -e ${cfg_file} || ! -e ${live_file} ]] ; then
 						MATCHES=0
 					else
@@ -377,17 +401,50 @@ do_file() {
 
 show_diff() {
 	clear
-	local file1=$1 file2=$2
+	local file1=$1 file2=$2 files=("$1" "$2") \
+		diff_files=() file i tmpdir
+
+	if [[ -L ${file1} && ! -L ${file2} &&
+		-f ${file1} && -f ${file2} ]] ; then
+		# If a regular file replaces a symlink to a regular file, then
+		# show the diff between the regular files (bug #330221).
+		diff_files=("${file1}" "${file2}")
+	else
+		for i in 0 1 ; do
+			if [[ ! -L ${files[$i]} && -f ${files[$i]} ]] ; then
+				diff_files[$i]=${files[$i]}
+				continue
+			fi
+			[[ -n ${tmpdir} ]] || \
+				tmpdir=$(mktemp -d "${TMP}/symdiff-XXX")
+			diff_files[$i]=${tmpdir}/${i}
+			if [[ ! -L ${files[$i]} && ! -e ${files[$i]} ]] ; then
+				echo "/dev/null" > "${diff_files[$i]}"
+			elif [[ -L ${files[$i]} ]] ; then
+				echo "SYM: ${file1} -> $(readlink "${files[$i]}")" > \
+					"${diff_files[$i]}"
+			elif [[ -d ${files[$i]} ]] ; then
+				echo "DIR: ${file1}" > "${diff_files[$i]}"
+			elif [[ -p ${files[$i]} ]] ; then
+				echo "FIF: ${file1}" > "${diff_files[$i]}"
+			else
+				echo "DEV: ${file1}" > "${diff_files[$i]}"
+			fi
+		done
+	fi
+
 	if [[ ${using_editor} == 0 ]] ; then
 		(
 			echo "Showing differences between ${file1} and ${file2}"
-			diff_command "${file1}" "${file2}"
+			diff_command "${diff_files[0]}" "${diff_files[1]}"
 		) | ${pager}
 	else
 		echo "Beginning of differences between ${file1} and ${file2}"
-		diff_command "${file1}" "${file2}"
+		diff_command "${diff_files[0]}" "${diff_files[1]}"
 		echo "End of differences between ${file1} and ${file2}"
 	fi
+
+	[[ -n ${tmpdir} ]] && rm -rf "${tmpdir}"
 }
 
 do_cfg() {
@@ -395,14 +452,14 @@ do_cfg() {
 	local ofile=$2
 	local -i my_input=0
 
-	until (( my_input == -1 )) || [ ! -f "${file}" ] ; do
+	until (( my_input == -1 )) || [[ ! -f ${file} && ! -L ${file} ]] ; do
 		if [[ "${OVERWRITE_ALL}" == "yes" ]] && ! user_special "${ofile}"; then
 			my_input=1
 		elif [[ "${DELETE_ALL}" == "yes" ]] && ! user_special "${ofile}"; then
 			my_input=2
 		else
 			show_diff "${ofile}" "${file}"
-			if [[ -L ${file} ]] ; then
+			if [[ -L ${file} && ! -L ${ofile} ]] ; then
 				cat <<-EOF
 
 					-------------------------------------------------------------
@@ -461,6 +518,19 @@ do_merge() {
 	local ofile="${2}"
 	local mfile="${TMP}/${2}.merged"
 	local -i my_input=0
+
+	if [[ -L ${file} && -L ${ofile} ]] ; then
+		echo "Both files are symlinks, so they will not be merged."
+		return 0
+	elif [[ ! -f ${file} ]] ; then
+		echo "Non-regular file cannot be merged: ${file}"
+		return 0
+	elif [[ ! -f ${ofile} ]] ; then
+		echo "Non-regular file cannot be merged: ${ofile}"
+		return 0
+	fi
+
+
 	echo "${file} ${ofile} ${mfile}"
 
 	if [[ -e ${mfile} ]] ; then
@@ -533,9 +603,18 @@ do_distconf() {
 	for (( count = 0; count <= 9999; ++count )) ; do
 		suffix=$(printf ".dist_%04i" ${count})
 		efile="${ofile}${suffix}"
-		if [[ ! -f ${efile} ]] ; then
+		if [[ ! -f ${efile} && ! -L ${efile} ]] ; then
 			mv ${mv_opts} "${file}" "${efile}"
 			break
+		elif [[ -L ${efile} && -L ${file} ]] ; then
+			if [[ $(readlink "${efile}") == $(readlink "${file}") ]] ; then
+				# replace identical copy
+				mv "${file}" "${efile}"
+				break
+			fi
+		elif [[ -L ${efile} || -L ${file} ]] ; then
+			# not the same file types
+			continue
 		elif diff_command "${file}" "${efile}" &> /dev/null; then
 			# replace identical copy
 			mv "${file}" "${efile}"
-- 
2.0.4



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

* [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: symlink support for bug #485598
  2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
                   ` (2 preceding siblings ...)
  2014-10-27 22:57 ` [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for " Zac Medico
@ 2014-10-27 23:04 ` Zac Medico
  2014-10-31 13:34 ` [gentoo-portage-dev] [PATCH] " Zac Medico
  4 siblings, 0 replies; 10+ messages in thread
From: Zac Medico @ 2014-10-27 23:04 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

This includes numerous logic adjustments that are needed to support
protected symlinks. The new diff_mixed function is used for diffs
between arbitrary file types. For example, a diff between two symlinks
looks like this:

-SYM: /foo/bar -> baz
+SYM: /foo/bar -> blah

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
This updated patch fixes diff_mixed to handle files that don't exist. These
are displayed as "/dev/null". Also, I renamed diffstatusoutput_symlink to
diffstatusoutput_mixed, since it works with mixed file types. Thanks to
Mike Gilbert <floppym@gentoo.org> for reporting the issue with files that
don't exist.

 bin/dispatch-conf            |  40 ++++++------
 pym/portage/dispatch_conf.py | 142 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 144 insertions(+), 38 deletions(-)

diff --git a/bin/dispatch-conf b/bin/dispatch-conf
index 6d2ae94..80dafd6 100755
--- a/bin/dispatch-conf
+++ b/bin/dispatch-conf
@@ -15,15 +15,15 @@ from __future__ import print_function
 
 from stat import ST_GID, ST_MODE, ST_UID
 from random import random
-import atexit, re, shutil, stat, sys
+import atexit, io, re, functools, shutil, sys
 from os import path as osp
 if osp.isfile(osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), ".portage_not_installed")):
 	sys.path.insert(0, osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), "pym"))
 import portage
 portage._internal_caller = True
 from portage import os
-from portage import _unicode_decode
-from portage.dispatch_conf import diffstatusoutput
+from portage import _encodings, _unicode_decode
+from portage.dispatch_conf import diffstatusoutput, diff_mixed
 from portage.process import find_binary, spawn
 
 FIND_EXTANT_CONFIGS  = "find '%s' %s -name '._cfg????_%s' ! -name '.*~' ! -iname '.*.bak' -print"
@@ -72,6 +72,11 @@ def cmd_var_is_valid(cmd):
 
     return find_binary(cmd[0]) is not None
 
+def diff(file1, file2):
+    return diff_mixed(
+        functools.partial(diffstatusoutput, DIFF_CONTENTS),
+        file1, file2)
+
 class dispatch:
     options = {}
 
@@ -89,8 +94,6 @@ class dispatch:
                or not os.path.exists(self.options["log-file"]):
                 open(self.options["log-file"], 'w').close() # Truncate it
                 os.chmod(self.options["log-file"], 0o600)
-        else:
-            self.options["log-file"] = "/dev/null"
 
         pager = self.options.get("pager")
         if pager is None or not cmd_var_is_valid(pager):
@@ -148,9 +151,6 @@ class dispatch:
             portage.util.shlex_split(
             portage.settings.get('CONFIG_PROTECT_MASK', '')))
 
-        def diff(file1, file2):
-            return diffstatusoutput(DIFF_CONTENTS, file1, file2)
-
         #
         # Remove new configs identical to current
         #                  and
@@ -166,7 +166,7 @@ class dispatch:
                 mrgfail = portage.dispatch_conf.rcs_archive(archive, conf['current'], conf['new'], mrgconf)
             else:
                 mrgfail = portage.dispatch_conf.file_archive(archive, conf['current'], conf['new'], mrgconf)
-            if os.path.exists(archive + '.dist'):
+            if os.path.lexists(archive + '.dist'):
                 unmodified = len(diff(conf['current'], archive + '.dist')[1]) == 0
             else:
                 unmodified = 0
@@ -181,7 +181,7 @@ class dispatch:
 
             if newconf == mrgconf and \
                 self.options.get('ignore-previously-merged') != 'yes' and \
-                os.path.exists(archive+'.dist') and \
+                os.path.lexists(archive+'.dist') and \
                 len(diff(archive+'.dist', conf['new'])[1]) == 0:
                 # The current update is identical to the archived .dist
                 # version that has previously been merged.
@@ -254,6 +254,11 @@ class dispatch:
 
         valid_input = "qhtnmlezu"
 
+        def diff_pager(file1, file2):
+            cmd = self.options['diff'] % (file1, file2)
+            cmd += pager
+            spawn_shell(cmd)
+
         for conf in confs:
             count = count + 1
 
@@ -266,14 +271,10 @@ class dispatch:
             while 1:
                 clear_screen()
                 if show_new_diff:
-                    cmd = self.options['diff'] % (conf['new'], mrgconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_mixed(diff_pager, conf['new'], mrgconf)
                     show_new_diff = 0
                 else:
-                    cmd = self.options['diff'] % (conf['current'], newconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_mixed(diff_pager, conf['current'], newconf)
 
                 print()
                 print('>> (%i of %i) -- %s' % (count, len(confs), conf ['current']))
@@ -357,7 +358,12 @@ class dispatch:
     def replace (self, newconf, curconf):
         """Replace current config with the new/merged version.  Also logs
         the diff of what changed into the configured log file."""
-        os.system((DIFF_CONTENTS % (curconf, newconf)) + '>>' + self.options["log-file"])
+        if "log-file" in self.options:
+            status, output = diff(curconf, newconf)
+            with io.open(self.options["log-file"], mode="a",
+                encoding=_encodings["stdio"]) as f:
+                f.write(output + "\n")
+
         try:
             os.rename(newconf, curconf)
         except (IOError, os.error) as why:
diff --git a/pym/portage/dispatch_conf.py b/pym/portage/dispatch_conf.py
index 113d965..d921c26 100644
--- a/pym/portage/dispatch_conf.py
+++ b/pym/portage/dispatch_conf.py
@@ -6,11 +6,12 @@
 # Library by Wayne Davison <gentoo@blorf.net>, derived from code
 # written by Jeremy Wohl (http://igmus.org)
 
-from __future__ import print_function
+from __future__ import print_function, unicode_literals
 
-import os, shutil, subprocess, sys
+import functools, io, os, shutil, stat, subprocess, sys, tempfile
 
 import portage
+from portage import _encodings
 from portage.env.loaders import KeyValuePairFileLoader
 from portage.localization import _
 from portage.util import shlex_split, varexpand
@@ -50,6 +51,58 @@ def diffstatusoutput(cmd, file1, file2):
 		output = output[:-1]
 	return (proc.wait(), output)
 
+def diff_mixed(func, file1, file2):
+	tempdir = None
+	try:
+		if os.path.islink(file1) and \
+			not os.path.islink(file2) and \
+			os.path.isfile(file1) and \
+			os.path.isfile(file2):
+			# If a regular file replaces a symlink to a regular
+			# file, then show the diff between the regular files
+			# (bug #330221).
+			diff_files = (file2, file2)
+		else:
+			files = [file1, file2]
+			diff_files = [file1, file2]
+			for i in range(len(diff_files)):
+				try:
+					st = os.lstat(diff_files[i])
+				except OSError:
+					st = None
+				if st is not None and stat.S_ISREG(st.st_mode):
+					continue
+
+				if tempdir is None:
+					tempdir = tempfile.mkdtemp()
+				diff_files[i] = os.path.join(tempdir, "%d" % i)
+				if st is None:
+					content = "/dev/null\n"
+				elif stat.S_ISLNK(st.st_mode):
+					link_dest = os.readlink(files[i])
+					content = "SYM: %s -> %s\n" % \
+						(file1, link_dest)
+				elif stat.S_ISDIR(st.st_mode):
+					content = "DIR: %s\n" % (file1,)
+				elif stat.S_ISFIFO(st.st_mode):
+					content = "FIF: %s\n" % (file1,)
+				else:
+					content = "DEV: %s\n" % (file1,)
+				with io.open(diff_files[i], mode='w',
+					encoding=_encodings['stdio']) as f:
+					f.write(content)
+
+		return func(diff_files[0], diff_files[1])
+
+	finally:
+		if tempdir is not None:
+			shutil.rmtree(tempdir)
+
+def diffstatusoutput_mixed(cmd, file1, file2):
+	return diff_mixed(
+		functools.partial(diffstatusoutput, cmd),
+		file1, file2)
+
 def read_config(mandatory_opts):
 	eprefix = portage.settings["EPREFIX"]
 	if portage._not_installed:
@@ -103,35 +156,57 @@ def rcs_archive(archive, curconf, newconf, mrgconf):
 	except OSError:
 		pass
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if os.path.exists(archive + ',v'):
+	if os.path.lexists(archive + ',v'):
 		os.system(RCS_LOCK + ' ' + archive)
 	os.system(RCS_PUT + ' ' + archive)
 
 	ret = 0
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		os.system(RCS_GET + ' -r' + RCS_BRANCH + ' ' + archive)
-		has_branch = os.path.exists(archive)
+		has_branch = os.path.lexists(archive)
 		if has_branch:
 			os.rename(archive, archive + '.dist')
 
 		try:
-			shutil.copy2(newconf, archive)
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), archive)
+			else:
+				shutil.copy2(newconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
 		if has_branch:
-			if mrgconf != '':
+			if mrgconf and os.path.isfile(archive) and \
+				os.path.isfile(mrgconf):
 				# This puts the results of the merge into mrgconf.
 				ret = os.system(RCS_MERGE % (archive, mrgconf))
-				mystat = os.lstat(newconf)
 				os.chmod(mrgconf, mystat.st_mode)
 				os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 		os.rename(archive, archive + '.dist.new')
@@ -153,10 +228,11 @@ def file_archive(archive, curconf, newconf, mrgconf):
 		pass
 
 	# Archive the current config file if it isn't already saved
-	if (os.path.exists(archive) and
-		len(diffstatusoutput("diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
+	if (os.path.lexists(archive) and
+		len(diffstatusoutput_mixed(
+		"diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
 		suf = 1
-		while suf < 9 and os.path.exists(archive + '.' + str(suf)):
+		while suf < 9 and os.path.lexists(archive + '.' + str(suf)):
 			suf += 1
 
 		while suf > 1:
@@ -165,26 +241,50 @@ def file_archive(archive, curconf, newconf, mrgconf):
 
 		os.rename(archive, archive + '.1')
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		# Save off new config file in the archive dir with .dist.new suffix
+		newconf_archive = archive + '.dist.new'
 		try:
-			shutil.copy2(newconf, archive + '.dist.new')
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), newconf_archive)
+			else:
+				shutil.copy2(newconf, newconf_archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive + '.dist.new', "reason": str(why)}, file=sys.stderr)
 
 		ret = 0
-		if mrgconf != '' and os.path.exists(archive + '.dist'):
+		if mrgconf and os.path.isfile(curconf) and \
+			os.path.isfile(newconf) and \
+			os.path.isfile(archive + '.dist'):
 			# This puts the results of the merge into mrgconf.
 			ret = os.system(DIFF3_MERGE % (curconf, archive + '.dist', newconf, mrgconf))
-			mystat = os.lstat(newconf)
 			os.chmod(mrgconf, mystat.st_mode)
 			os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 
@@ -195,7 +295,7 @@ def rcs_archive_post_process(archive):
 	"""Check in the archive file with the .dist.new suffix on the branch
 	and remove the one with the .dist suffix."""
 	os.rename(archive + '.dist.new', archive)
-	if os.path.exists(archive + '.dist'):
+	if os.path.lexists(archive + '.dist'):
 		# Commit the last-distributed version onto the branch.
 		os.system(RCS_LOCK + RCS_BRANCH + ' ' + archive)
 		os.system(RCS_PUT + ' -r' + RCS_BRANCH + ' ' + archive)
@@ -207,5 +307,5 @@ def rcs_archive_post_process(archive):
 
 def file_archive_post_process(archive):
 	"""Rename the archive file with the .dist.new suffix to a .dist suffix"""
-	if os.path.exists(archive + '.dist.new'):
+	if os.path.lexists(archive + '.dist.new'):
 		os.rename(archive + '.dist.new', archive + '.dist')
-- 
2.0.4



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

* [gentoo-portage-dev] [PATCH] dispatch-conf: symlink support for bug #485598
  2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
                   ` (3 preceding siblings ...)
  2014-10-27 23:04 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " Zac Medico
@ 2014-10-31 13:34 ` Zac Medico
  4 siblings, 0 replies; 10+ messages in thread
From: Zac Medico @ 2014-10-31 13:34 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

This includes numerous logic adjustments that are needed to support
protected symlinks. The new diff_mixed function is used for diffs
between arbitrary file types. For example, a diff between two symlinks
looks like this:

-SYM: /foo/bar -> baz
+SYM: /foo/bar -> blah

X-Gentoo-Bug: 485598
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
---
This updated patch adds a diff_mixed_wrapper class which is used to simplify
diff_mixed usage.

 bin/dispatch-conf            |  39 ++++++-----
 pym/portage/dispatch_conf.py | 150 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 151 insertions(+), 38 deletions(-)

diff --git a/bin/dispatch-conf b/bin/dispatch-conf
index 6d2ae94..412dcdc 100755
--- a/bin/dispatch-conf
+++ b/bin/dispatch-conf
@@ -15,15 +15,15 @@ from __future__ import print_function
 
 from stat import ST_GID, ST_MODE, ST_UID
 from random import random
-import atexit, re, shutil, stat, sys
+import atexit, io, re, functools, shutil, sys
 from os import path as osp
 if osp.isfile(osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), ".portage_not_installed")):
 	sys.path.insert(0, osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), "pym"))
 import portage
 portage._internal_caller = True
 from portage import os
-from portage import _unicode_decode
-from portage.dispatch_conf import diffstatusoutput
+from portage import _encodings, _unicode_decode
+from portage.dispatch_conf import diffstatusoutput, diff_mixed_wrapper
 from portage.process import find_binary, spawn
 
 FIND_EXTANT_CONFIGS  = "find '%s' %s -name '._cfg????_%s' ! -name '.*~' ! -iname '.*.bak' -print"
@@ -72,6 +72,8 @@ def cmd_var_is_valid(cmd):
 
     return find_binary(cmd[0]) is not None
 
+diff = diff_mixed_wrapper(diffstatusoutput, DIFF_CONTENTS)
+
 class dispatch:
     options = {}
 
@@ -89,8 +91,6 @@ class dispatch:
                or not os.path.exists(self.options["log-file"]):
                 open(self.options["log-file"], 'w').close() # Truncate it
                 os.chmod(self.options["log-file"], 0o600)
-        else:
-            self.options["log-file"] = "/dev/null"
 
         pager = self.options.get("pager")
         if pager is None or not cmd_var_is_valid(pager):
@@ -148,9 +148,6 @@ class dispatch:
             portage.util.shlex_split(
             portage.settings.get('CONFIG_PROTECT_MASK', '')))
 
-        def diff(file1, file2):
-            return diffstatusoutput(DIFF_CONTENTS, file1, file2)
-
         #
         # Remove new configs identical to current
         #                  and
@@ -166,7 +163,7 @@ class dispatch:
                 mrgfail = portage.dispatch_conf.rcs_archive(archive, conf['current'], conf['new'], mrgconf)
             else:
                 mrgfail = portage.dispatch_conf.file_archive(archive, conf['current'], conf['new'], mrgconf)
-            if os.path.exists(archive + '.dist'):
+            if os.path.lexists(archive + '.dist'):
                 unmodified = len(diff(conf['current'], archive + '.dist')[1]) == 0
             else:
                 unmodified = 0
@@ -181,7 +178,7 @@ class dispatch:
 
             if newconf == mrgconf and \
                 self.options.get('ignore-previously-merged') != 'yes' and \
-                os.path.exists(archive+'.dist') and \
+                os.path.lexists(archive+'.dist') and \
                 len(diff(archive+'.dist', conf['new'])[1]) == 0:
                 # The current update is identical to the archived .dist
                 # version that has previously been merged.
@@ -254,6 +251,13 @@ class dispatch:
 
         valid_input = "qhtnmlezu"
 
+        def diff_pager(file1, file2):
+            cmd = self.options['diff'] % (file1, file2)
+            cmd += pager
+            spawn_shell(cmd)
+
+        diff_pager = diff_mixed_wrapper(diff_pager)
+
         for conf in confs:
             count = count + 1
 
@@ -266,14 +270,10 @@ class dispatch:
             while 1:
                 clear_screen()
                 if show_new_diff:
-                    cmd = self.options['diff'] % (conf['new'], mrgconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_pager(conf['new'], mrgconf)
                     show_new_diff = 0
                 else:
-                    cmd = self.options['diff'] % (conf['current'], newconf)
-                    cmd += pager
-                    spawn_shell(cmd)
+                    diff_pager(conf['current'], newconf)
 
                 print()
                 print('>> (%i of %i) -- %s' % (count, len(confs), conf ['current']))
@@ -357,7 +357,12 @@ class dispatch:
     def replace (self, newconf, curconf):
         """Replace current config with the new/merged version.  Also logs
         the diff of what changed into the configured log file."""
-        os.system((DIFF_CONTENTS % (curconf, newconf)) + '>>' + self.options["log-file"])
+        if "log-file" in self.options:
+            status, output = diff(curconf, newconf)
+            with io.open(self.options["log-file"], mode="a",
+                encoding=_encodings["stdio"]) as f:
+                f.write(output + "\n")
+
         try:
             os.rename(newconf, curconf)
         except (IOError, os.error) as why:
diff --git a/pym/portage/dispatch_conf.py b/pym/portage/dispatch_conf.py
index 113d965..cda45d5 100644
--- a/pym/portage/dispatch_conf.py
+++ b/pym/portage/dispatch_conf.py
@@ -6,11 +6,12 @@
 # Library by Wayne Davison <gentoo@blorf.net>, derived from code
 # written by Jeremy Wohl (http://igmus.org)
 
-from __future__ import print_function
+from __future__ import print_function, unicode_literals
 
-import os, shutil, subprocess, sys
+import functools, io, os, shutil, stat, subprocess, sys, tempfile
 
 import portage
+from portage import _encodings
 from portage.env.loaders import KeyValuePairFileLoader
 from portage.localization import _
 from portage.util import shlex_split, varexpand
@@ -50,6 +51,66 @@ def diffstatusoutput(cmd, file1, file2):
 		output = output[:-1]
 	return (proc.wait(), output)
 
+def diff_mixed(func, file1, file2):
+	tempdir = None
+	try:
+		if os.path.islink(file1) and \
+			not os.path.islink(file2) and \
+			os.path.isfile(file1) and \
+			os.path.isfile(file2):
+			# If a regular file replaces a symlink to a regular
+			# file, then show the diff between the regular files
+			# (bug #330221).
+			diff_files = (file2, file2)
+		else:
+			files = [file1, file2]
+			diff_files = [file1, file2]
+			for i in range(len(diff_files)):
+				try:
+					st = os.lstat(diff_files[i])
+				except OSError:
+					st = None
+				if st is not None and stat.S_ISREG(st.st_mode):
+					continue
+
+				if tempdir is None:
+					tempdir = tempfile.mkdtemp()
+				diff_files[i] = os.path.join(tempdir, "%d" % i)
+				if st is None:
+					content = "/dev/null\n"
+				elif stat.S_ISLNK(st.st_mode):
+					link_dest = os.readlink(files[i])
+					content = "SYM: %s -> %s\n" % \
+						(file1, link_dest)
+				elif stat.S_ISDIR(st.st_mode):
+					content = "DIR: %s\n" % (file1,)
+				elif stat.S_ISFIFO(st.st_mode):
+					content = "FIF: %s\n" % (file1,)
+				else:
+					content = "DEV: %s\n" % (file1,)
+				with io.open(diff_files[i], mode='w',
+					encoding=_encodings['stdio']) as f:
+					f.write(content)
+
+		return func(diff_files[0], diff_files[1])
+
+	finally:
+		if tempdir is not None:
+			shutil.rmtree(tempdir)
+
+class diff_mixed_wrapper(object):
+
+	def __init__(self, f, *args):
+		self._func = f
+		self._args = args
+
+	def __call__(self, *args):
+		return diff_mixed(
+			functools.partial(self._func, *(self._args + args[:-2])),
+			*args[-2:])
+
+diffstatusoutput_mixed = diff_mixed_wrapper(diffstatusoutput)
+
 def read_config(mandatory_opts):
 	eprefix = portage.settings["EPREFIX"]
 	if portage._not_installed:
@@ -103,35 +164,57 @@ def rcs_archive(archive, curconf, newconf, mrgconf):
 	except OSError:
 		pass
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if os.path.exists(archive + ',v'):
+	if os.path.lexists(archive + ',v'):
 		os.system(RCS_LOCK + ' ' + archive)
 	os.system(RCS_PUT + ' ' + archive)
 
 	ret = 0
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		os.system(RCS_GET + ' -r' + RCS_BRANCH + ' ' + archive)
-		has_branch = os.path.exists(archive)
+		has_branch = os.path.lexists(archive)
 		if has_branch:
 			os.rename(archive, archive + '.dist')
 
 		try:
-			shutil.copy2(newconf, archive)
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), archive)
+			else:
+				shutil.copy2(newconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
 		if has_branch:
-			if mrgconf != '':
+			if mrgconf and os.path.isfile(archive) and \
+				os.path.isfile(mrgconf):
 				# This puts the results of the merge into mrgconf.
 				ret = os.system(RCS_MERGE % (archive, mrgconf))
-				mystat = os.lstat(newconf)
 				os.chmod(mrgconf, mystat.st_mode)
 				os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 		os.rename(archive, archive + '.dist.new')
@@ -153,10 +236,11 @@ def file_archive(archive, curconf, newconf, mrgconf):
 		pass
 
 	# Archive the current config file if it isn't already saved
-	if (os.path.exists(archive) and
-		len(diffstatusoutput("diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
+	if (os.path.lexists(archive) and
+		len(diffstatusoutput_mixed(
+		"diff -aq '%s' '%s'", curconf, archive)[1]) != 0):
 		suf = 1
-		while suf < 9 and os.path.exists(archive + '.' + str(suf)):
+		while suf < 9 and os.path.lexists(archive + '.' + str(suf)):
 			suf += 1
 
 		while suf > 1:
@@ -165,26 +249,50 @@ def file_archive(archive, curconf, newconf, mrgconf):
 
 		os.rename(archive, archive + '.1')
 
-	if os.path.isfile(curconf):
+	try:
+		curconf_st = os.lstat(curconf)
+	except OSError:
+		curconf_st = None
+
+	if curconf_st is not None and \
+		(stat.S_ISREG(curconf_st.st_mode) or
+		stat.S_ISLNK(curconf_st.st_mode)):
 		try:
-			shutil.copy2(curconf, archive)
+			if stat.S_ISLNK(curconf_st.st_mode):
+				os.symlink(os.readlink(curconf), archive)
+			else:
+				shutil.copy2(curconf, archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(curconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"curconf": curconf, "archive": archive, "reason": str(why)}, file=sys.stderr)
 
-	if newconf != '':
+	mystat = None
+	if newconf:
+		try:
+			mystat = os.lstat(newconf)
+		except OSError:
+			pass
+
+	if mystat is not None and \
+		(stat.S_ISREG(mystat.st_mode) or
+		stat.S_ISLNK(mystat.st_mode)):
 		# Save off new config file in the archive dir with .dist.new suffix
+		newconf_archive = archive + '.dist.new'
 		try:
-			shutil.copy2(newconf, archive + '.dist.new')
+			if stat.S_ISLNK(mystat.st_mode):
+				os.symlink(os.readlink(newconf), newconf_archive)
+			else:
+				shutil.copy2(newconf, newconf_archive)
 		except(IOError, os.error) as why:
 			print(_('dispatch-conf: Error copying %(newconf)s to %(archive)s: %(reason)s; fatal') % \
 				{"newconf": newconf, "archive": archive + '.dist.new', "reason": str(why)}, file=sys.stderr)
 
 		ret = 0
-		if mrgconf != '' and os.path.exists(archive + '.dist'):
+		if mrgconf and os.path.isfile(curconf) and \
+			os.path.isfile(newconf) and \
+			os.path.isfile(archive + '.dist'):
 			# This puts the results of the merge into mrgconf.
 			ret = os.system(DIFF3_MERGE % (curconf, archive + '.dist', newconf, mrgconf))
-			mystat = os.lstat(newconf)
 			os.chmod(mrgconf, mystat.st_mode)
 			os.chown(mrgconf, mystat.st_uid, mystat.st_gid)
 
@@ -195,7 +303,7 @@ def rcs_archive_post_process(archive):
 	"""Check in the archive file with the .dist.new suffix on the branch
 	and remove the one with the .dist suffix."""
 	os.rename(archive + '.dist.new', archive)
-	if os.path.exists(archive + '.dist'):
+	if os.path.lexists(archive + '.dist'):
 		# Commit the last-distributed version onto the branch.
 		os.system(RCS_LOCK + RCS_BRANCH + ' ' + archive)
 		os.system(RCS_PUT + ' -r' + RCS_BRANCH + ' ' + archive)
@@ -207,5 +315,5 @@ def rcs_archive_post_process(archive):
 
 def file_archive_post_process(archive):
 	"""Rename the archive file with the .dist.new suffix to a .dist suffix"""
-	if os.path.exists(archive + '.dist.new'):
+	if os.path.lexists(archive + '.dist.new'):
 		os.rename(archive + '.dist.new', archive + '.dist')
-- 
2.0.4



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

* Re: [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, bug #485598
  2014-10-27 20:35     ` Zac Medico
@ 2014-11-03  3:54       ` Brian Dolbec
  0 siblings, 0 replies; 10+ messages in thread
From: Brian Dolbec @ 2014-11-03  3:54 UTC (permalink / raw
  To: gentoo-portage-dev

On Mon, 27 Oct 2014 13:35:19 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> Users may not want some symlinks to get clobbered, so protect them
> with CONFIG_PROTECT. Changes were required in the dblink.mergeme
> method and the new_protect_filename function.
> 
> The changes to dblink.mergeme do 3 things:
> 
>  * Move the bulk of config protection logic from dblink.mergeme to a
>    new dblink._protect method. The new method only returns 3
> variables, which makes it easier to understand how config protection
> interacts with the dblink.mergeme code that uses those variables.
> This is important, since dblink.mergeme has so many variables.
> 
>  * Initialize more variables at the beginning of dblink.mergeme, since
>    those variables are used by the dblink._protect method.
> 
>  * Use the variables returned from dblink._protect to trigger
>    appropriate behavior later in dblink.mergeme.
> 
> The new_protect_filename changes are required since this function
> compares the new file to old ._cfg* files that may already exist, in
> order to avoid creating duplicate ._cfg* files. In these comparisons,
> it needs to handle symlinks differently from regular files.
> 
> The unit tests demonstrate operation in many different scenarios,
> including:
> 
>  * regular file replaces regular file
>  * regular file replaces symlink
>  * regular file replaces directory
>  * symlink replaces symlink
>  * symlink replaces regular file
>  * symlink replaces directory
>  * directory replaces regular file
>  * directory replaces symlink
> 
> X-Gentoo-Bug: 485598
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=485598
> ---
> This updated patch only adds to the commit message in order to
> provide some information that may be helpful to reviewers of the
> patch. There are no changes to the code.
> 
>  pym/portage/dbapi/vartree.py                    | 255
> ++++++++++++--------- pym/portage/tests/emerge/test_config_protect.py
> | 292 ++++++++++++++++++++++++
> pym/portage/util/__init__.py                    |  35 ++- 3 files
> changed, 463 insertions(+), 119 deletions(-) create mode 100644
> pym/portage/tests/emerge/test_config_protect.py
> 

See my github inline comments re: use of my* variables


-- 
Brian Dolbec <dolsen>



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

end of thread, other threads:[~2014-11-03  3:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-26 11:12 [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for bug #485598 zmedico
2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " zmedico
2014-10-26 11:12 ` [gentoo-portage-dev] [PATCH 3/3] CONFIG_PROTECT: protect symlinks, " zmedico
2014-10-27  8:08   ` Alexander Berntsen
2014-10-27  9:07     ` Zac Medico
2014-10-27 20:35     ` Zac Medico
2014-11-03  3:54       ` Brian Dolbec
2014-10-27 22:57 ` [gentoo-portage-dev] [PATCH 1/3] etc-update: symlink support for " Zac Medico
2014-10-27 23:04 ` [gentoo-portage-dev] [PATCH 2/3] dispatch-conf: " Zac Medico
2014-10-31 13:34 ` [gentoo-portage-dev] [PATCH] " Zac Medico

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