[apparmor] new profile tools - review of merging branch

[Christian Boltz]

Hello,

the attached files contain my review notes for the merging branch
lp:~sbeattie/apparmor/apparmor-new-pyutils-branch/
but they only contain some comments.

I didn't find something terribly wrong, so I'd say:
For merging this branch (r2392 to be exact):
Acked-by: Christian Boltz <apparmor at cboltz.de>


I also noticed my patches
- new profile tools: preserve full initial comment
- new profile tools - handling of "(F)inish"
are not included yet. Can you please review and (hopefully) merge them?


Regards,

Christian Boltz
-- 
PATH="${HOME}/Oktoberfest 2003:$PATH"
configure '--prefix=Auf geht\'s'
[Ralf Corsepius in suse-programming]
-------------- next part --------------
------------------------------------------------------------
revno: 2372 [merge]
committer: Steve Beattie <sbeattie at ubuntu.com>
branch nick: apparmor-new-pyutils-branch
timestamp: Wed 2014-02-12 15:54:00 -0800
message:
  Merge in Kshitij Gupta <kgupta8592 at gmail.com>'s rewrite of the
  logprof/genprof and related utilities in python. Because the branch that
  was worked on was not based on the apparmor tree, not all of the history
  can be maintained for files that are not newly created or entirely
  rewritten in the branch.
  
  (This merge also includes a subsequent commit to the branch
  I was merging from which includes my missed bzr add of
  utils/apparmor/translations.py)
------------------------------------------------------------
Use --include-merged or -n0 to see merged revisions.






+++ utils/apparmor/translations.py 
+# ------------------------------------------------------------------
+#
+#    Copyright (C) 2014 Canonical Ltd.
+#
+#    This program is free software; you can redistribute it and/or
+#    modify it under the terms of version 2 of the GNU General Public
+#    License published by the Free Software Foundation.
+#
+# ------------------------------------------------------------------
+import gettext
+
+TRANSLATION_DOMAIN = 'apparmor-utils'
+
+__apparmor_gettext__ = None
+
+def init_translation():
+    global __apparmor_gettext__
+    if __apparmor_gettext__ is None:
+        t = gettext.translation(TRANSLATION_DOMAIN, fallback=True)
+        __apparmor_gettext__ = t.gettext
+    return __apparmor_gettext__

# what's the reason to make __apparmor_gettext__ global?



cb at geeko:~/apparmor/sbeattie-gsoc-pre-merge/apparmor-new-pyutils-branch/utils> diff -r test/ ~/apparmor/sbeattie-gsoc-pre-merge/pre-merger-cleanups/Testing/
diff -u -p -r test/easyprof.conf /home/cb/apparmor/sbeattie-gsoc-pre-merge/pre-merger-cleanups/Testing/easyprof.conf
--- test/easyprof.conf  2014-02-14 21:01:40.081542000 +0100
+++ /home/cb/apparmor/sbeattie-gsoc-pre-merge/pre-merger-cleanups/Testing/easyprof.conf 2014-02-11 18:48:23.035073000 +0100
@@ -1,5 +1,5 @@
 # Location of system policygroups
-POLICYGROUPS_DIR="./policygroups"
+POLICYGROUPS_DIR="/usr/share/apparmor/easyprof/policygroups"
 
 # Location of system templates
-TEMPLATES_DIR="./templates"
+TEMPLATES_DIR="/usr/share/apparmor/easyprof/templates"


# I'd understand this change for the system-wide config, but in /test/ ?



vim:ft=diff
-------------- next part --------------
------------------------------------------------------------
revno: 2380
committer: Steve Beattie <sbeattie at ubuntu.com>
branch nick: apparmor-new-pyutils-branch
timestamp: Thu 2014-02-13 10:52:00 -0800
message:
  utils/apparmor/: work around last of pyflakes issues here


=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py	2014-02-13 18:01:03 +0000
+++ utils/apparmor/aa.py	2014-02-13 18:52:00 +0000
@@ -1088,7 +1088,7 @@
                         context_new = context_new + '^%s' % hat
                     context_new = context_new + ' -> %s' % exec_target
 
-                    ans_new = transitions.get(context_new, '')
+                    # ans_new = transitions.get(context_new, '')  # XXX ans meant here?

# good question ;-)

                     combinedmode = set()
                     combinedaudit = set()
                     ## Check return Value Consistency
@@ -2188,9 +2188,9 @@
 
 def do_logprof_pass(logmark='', passno=0, pid=pid):
 ...
-    seen = hasher()
+#    seen = hasher()  # XXX global?

# maybe - I'd guess the intention here is to reset it to empty
# (might be needed for genprof, which can do several passes)

@@ -2201,7 +2201,7 @@
     log = []
 #     log_dict = hasher()
 #     changed = dict()
-    skip = hasher()
+#    skip = hasher()  # XXX global?

# maybe - I'd guess the intention here is to reset it to empty
# (might be needed for genprof, which can do several passes)

@@ -3402,7 +3401,7 @@
                          'path': write_paths,
                          'change_profile': write_change_profile,
                          }
-        prof_correct = True
+        # prof_correct = True  # XXX correct?

# maybe, but not needed because the for loop starts with "correct = true"

         segments = {'alias': False,
                     'lvar': False,
                     'include': False,

=== modified file 'utils/apparmor/yasti.py'
--- utils/apparmor/yasti.py	2014-02-13 18:01:03 +0000
+++ utils/apparmor/yasti.py	2014-02-13 18:52:00 +0000
@@ -38,7 +38,7 @@
         ycommand, ypath, yargument = ParseCommand(line)
         if ycommand and ycommand == 'Read':
             debug_logger.info('SendDataToYast: Sending--%s' % data)
-            Return(data)
+            ycp.Return(data)  # XXX is this right?

# yasti.py is still unused, so you can't break much ;-)
# (same for the other two XXX in yasti.py)


vim:ft=diff