Thanks Saurabh and Shawn for the review.
I have attached the patch for CR 7139743.
Could you please integrate it.
Abhi.
On 09/24/12 15:12, Saurabh Vyas wrote:
Hi Abhi,
These changes LGTM.
Regards,
Saurabh
On 09/24/12 14:27, Abhinandan Ekande wrote:
Folks,
Per the latest webrev, for license action when hash attribute is missing
the
following error would be printed.
pkgsend: invalid action, 'license NOHASH license=copyright': Empty
or invalid path attribute
I had offline conversation with Shawn about this error message and it is
acceptable to him. Please let me know your opinion about this error
message.
Thanks,
Abhi.
On 09/14/12 16:16, Abhinandan Ekande wrote:
Shawn,
On 09/13/12 23:18, Shawn Walker wrote:
src/modules/actions/__init__.py:
lines 348-352: In general, when writing things in Python, the
preferred approach is to "leap first, then look". That is,
don't check for a key or value, simply perform the action and
catch the exception raised and handle it. In addition, the
exception raised here assumes that because there is no "path"
that this is a license action. That's not necessarily true.
Thanks for the feedback.
So instead of:
if "path" in action.attrs and payload == "NOHASH":
filepath = os.path.sep + action.attrs["path"]
elif "path" not in action.attrs and payload == "NOHASH":
raise ActionDataError(_("Hash attribute is missing "
"for license action"))
...I'd try this:
if payload == "NOHASH":
try:
filepath = os.path.sep + action.attrs["path"]
except KeyError:
raise InvalidPathAttributeError(action)
I followed the above approach. Here is updated webrev :
https://cr.opensolaris.org/action/browse/pkg/ae112802/7139743-rev2/webrev/
src/modules/publish/dependencies.py:
I don't think this change is needed if you do the above.
Yes, not needed with new approach.
Abhi.
We may need to tweak this, so can you try that and see what you get?
-Shawn
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
# HG changeset patch
# User abhinandan.eka...@oracle.com
# Date 1348565222 -19800
# Node ID 3ddcefe9124de9973b4c94468ecc1696229ef212
# Parent 0358e521cc2d9658d2a8d8ccd92112f2605a12ab
7139743 license actions without a path can cause a stacktrace
diff -r 0358e521cc2d -r 3ddcefe9124d src/modules/actions/__init__.py
--- a/src/modules/actions/__init__.py Fri Sep 14 10:29:11 2012 +0530
+++ b/src/modules/actions/__init__.py Tue Sep 25 14:57:02 2012 +0530
@@ -346,7 +346,10 @@
return None, None
if payload == "NOHASH":
- filepath = os.path.sep + action.attrs["path"]
+ try:
+ filepath = os.path.sep + action.attrs["path"]
+ except KeyError:
+ raise InvalidPathAttributeError(action)
else:
filepath = payload
diff -r 0358e521cc2d -r 3ddcefe9124d src/tests/cli/t_pkgsend.py
--- a/src/tests/cli/t_pkgsend.py Fri Sep 14 10:29:11 2012 +0530
+++ b/src/tests/cli/t_pkgsend.py Tue Sep 25 14:57:02 2012 +0530
@@ -1269,6 +1269,17 @@
self.pkgsend("", "-s %s publish %s" %
(self.dc.get_repo_url(), mfpath), exit=1)
+ def test_25_pkgsend_publish_nohash_license(self):
+ """Verify that publishing a manifest with hash attribute
+ missing for license action doesn't traceback"""
+
+ durl = self.dc.get_depot_url()
+ # Should fail because hash attribute is missing.
+ self.pkgsend_bulk(durl,
+ """open foo@1.0
+ add license license=copyright
+ close""", exit=1)
+
class TestPkgsendHardlinks(pkg5unittest.CliTestCase):
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
