REVIEW: use of python svn 1.0.1

[mscheinert]

Describe the bug
It seems the svn module (svn.local) is only used to import the exceptions svn.exception.SvnException after a subprocess call, e.g.:

nemo-simsar/src/mkReadme.py

Lines 377 to 388 in 9377f51

	if (LNGIT == False):
	try:
	#repo = subprocess.check_output("svn info | awk '/^Kind:/ {print $2}'", shell=True).strip()
	repo = subprocess.check_output("LC_ALL=C svn info | awk '/^Kind:/ {print $2}'", shell=True).strip()
	except svn.exception.SvnException:
	print(BLUE + "This is not an svn repo \n" + NORMAL)
	except OSError as e:
	print("Error - ", e)
	else:
	print(BLUE + "This is an SVN Repo" + NORMAL)
	global LNSVN
	LNSVN=True

If we find a workaround for this, we could drop the svn module (GPL-2) completely...

Additional context
The svn python package is distributed under GPL-2 which has implications for products linking to this library! (It's not the LGPL!)

[mscheinert]

Is this working at all? @lmicallef , please could you verify that?

The only case, where it would make sense is in check_gitorsvn():

nemo-simsar/src/mkReadme.py

Lines 377 to 388 in 9377f51

	if (LNGIT == False):
	try:
	#repo = subprocess.check_output("svn info | awk '/^Kind:/ {print $2}'", shell=True).strip()
	repo = subprocess.check_output("LC_ALL=C svn info | awk '/^Kind:/ {print $2}'", shell=True).strip()
	except svn.exception.SvnException:
	print(BLUE + "This is not an svn repo \n" + NORMAL)
	except OSError as e:
	print("Error - ", e)
	else:
	print(BLUE + "This is an SVN Repo" + NORMAL)
	global LNSVN
	LNSVN=True

where we have to check, whether the svn command raises an error or not. If repo.returncode > 0 then there is a problem with the working copy and it's therefore not a valid svn repo.
But it schould be sufficient to replace the SvnException with a subprocess.CalledProcessError and make sure, the error-message comes from svn. Something like this:

try:
    subprocess.check_output("svn info", stderr=subprocess.STDOUT, shell=True)
except subprocess.CalledProcessError as e:
    if e.output.decode("utf-8").startswith('svn: '):
        print(BLUE + "This is not an svn repo \n" + NORMAL)
    else:
        print("Error - ", e)
except OSError as e: 
    print("Error - ", e)
else:
    print(BLUE + "This is an SVN Repo" + NORMAL)

The exceptions in get_currepo():

nemo-simsar/src/mkReadme.py

Lines 569 to 589 in 9377f51

	if(LNSVN == True):
	try:
	#MMS default = subprocess.check_output("svn info | awk '/^URL:/ {print $2}'", shell=True).strip()
	default = subprocess.check_output("LC_ALL=C svn info --show-item url", shell=True).strip()
	revision = subprocess.check_output("LC_ALL=C svn info --show-item revision", shell=True).strip().decode().replace("'", "")
	#convert to string
	default = str(default)
	default = default.replace("'", "")
	if revision == "":
	print(RED + "SVN repository found, but current directory has not been committed" + NORMAL)
	elif(default != "") & (LNSVN == True) & (revision != ""):
	print(PURPLE + "This is an SVN Repo " + NORMAL)
	print(PURPLE + "Actual Repository = " + str(default[1:]) + NORMAL)
	return default[1:]
	else:
	print(RED + "Not connected to SVN" + NORMAL)
	except svn.exception.SvnException:
	print("This is not a working copy of SVN - Not connected to SVN")
	else:
	pass

and get_nemorevision():

nemo-simsar/src/mkReadme.py

Lines 636 to 645 in 9377f51

	if(LNSVN==True):
	try:
	#MMS nemoRev = subprocess.check_output("svn info | awk '/^Revision:/ {print $2}'", shell=True).strip()
	nemoRev = subprocess.check_output("LC_ALL=C svn info --show-item revision $(LC_ALL=C svn info --show-item wc-root)", shell=True).strip()
	nemoRev = str(nemoRev)
	nemoRev = nemoRev.replace("'", "")
	print(PURPLE + str(nemoRev[1:]) + NORMAL)
	return(nemoRev[1:])
	except svn.exception.SvnException:
	print("This is not an svn repo \n")

could be handled in a similar way using the subprocess.CalledProcessError exception but wihtout a svn specific test.

[lmicallef]

@mscheinert, I tested subprocess.CalledProcessError in check_gitorsvn() and it does work. I will work on get_currepo() and get_nemorevision() to include subprocess.calledProcessError instead of svn.exception.SvnException. If the latter works too, I agree we can remove the svn.local module.

[lmicallef]

get_currepo() and get_nemorevision() seem to working fine with subprocess.calledProcessError. We can eliminate svn.local module.

[lmicallef]

The latest script reflects this change.