Bug 874246

Summary: review-env.sh: '}': not a valid identifier
Product: [Fedora] Fedora Reporter: Jerry James <loganjerry>
Component: fedora-reviewAssignee: Stanislav Ochotnicky <sochotni>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 17CC: leamas.alec, pingou, sochotni
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-11-26 01:52:18 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jerry James 2012-11-07 19:06:34 UTC
Description of problem:
I just ran fedora-review and saw this output:

Illegal return from /usr/lib/python2.7/site-packages/FedoraReview/scripts/check-large-data.sh, code 80, output: stdout:None stderr:./review-env.sh: line 2: unset: `}': not a valid identifier

Illegal return from /usr/lib/python2.7/site-packages/FedoraReview/scripts/check-src-opt.sh, code 80, output: stdout:None stderr:./review-env.sh: line 2: unset: `}': not a valid identifier

This is line 2 of the generated review-env.sh file:

unset $(env | sed 's/=.*//')

Sure enough, if I run "env", I see this:

module=() {  eval `/usr/bin/modulecmd bash $*`
}

on two separate lines.  I think that fedora-review can and should avoid such junk in the environment.  Changing that line in review-env.sh to this works:

unset $(env | sed -n 's/=.*//p')

Version-Release number of selected component (if applicable):
fedora-review-0.3.1-2.fc17.noarch

How reproducible:
Always (in my environment).

Steps to Reproduce:
1. Have a function definition in the environment, such as the module() definition above, that contains embedded newlines, so that the closing curly brace is on a line by itself.
2. Run fedora-review
  
Actual results:
fedora-review spews complaints about the curly brace.

Expected results:
fedora-review should ignore the curly brace.

Additional info:

Comment 1 Alec Leamas 2012-11-07 20:32:23 UTC
Many thanks for report and, not least, some sed wizardry. This is now in devel:
https://fedorahosted.org/FedoraReview/changeset/9f9c261ab1a7741da0cbe436b67088224b84897a

However, it might take some time before there's a new release, we just shipped 3.1.1. If this bug is annoying enough for you to do something you could either try the devel version as described in https://fedorahosted.org/FedoraReview/wiki or apply the patch below to current f-r (you find the 
shell_api.py in /usr/share/fedora-review/plugins)

Once again: thanks!


diff --git a/plugins/shell_api.py b/plugins/shell_api.py
index 4761386..6d21d02 100644
--- a/plugins/shell_api.py
+++ b/plugins/shell_api.py
@@ -32,7 +32,7 @@ from FedoraReview import AbstractRegistry, GenericCheck
 from FedoraReview import ReviewDirs, Settings, XdgDirs
 
 ENVIRON_TEMPLATE = """
-unset $(env | sed 's/=.*//')
+unset $(env | sed -n 's/=.*//p')
 PATH=/bin:/usr/bin:/sbin/:/usr/sbin
 
 FR_FLAGS_generator

Comment 2 Fedora Update System 2012-11-16 15:12:55 UTC
fedora-review-0.3.1-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/fedora-review-0.3.1-3.fc17

Comment 3 Fedora Update System 2012-11-16 15:13:08 UTC
fedora-review-0.3.1-3.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/fedora-review-0.3.1-3.el6

Comment 4 Fedora Update System 2012-11-16 15:13:34 UTC
fedora-review-0.3.1-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/fedora-review-0.3.1-3.fc18

Comment 5 Fedora Update System 2012-11-16 15:13:49 UTC
fedora-review-0.3.1-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/fedora-review-0.3.1-3.fc16

Comment 6 Fedora Update System 2012-11-17 01:05:17 UTC
Package fedora-review-0.3.1-3.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=epel-testing fedora-review-0.3.1-3.el6'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-EPEL-2012-13487/fedora-review-0.3.1-3.el6
then log in and leave karma (feedback).

Comment 7 Fedora Update System 2012-11-26 01:52:20 UTC
fedora-review-0.3.1-3.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 8 Fedora Update System 2012-11-26 01:52:56 UTC
fedora-review-0.3.1-3.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 9 Fedora Update System 2012-11-27 04:39:05 UTC
fedora-review-0.3.1-3.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2012-12-11 20:29:12 UTC
fedora-review-0.3.1-3.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.