Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#1274 closed defect (fixed)

config_plugins installation location and python byte compilation

Reported by: jreed Owned by: UnAssigned
Priority: low Milestone: Sprint-20111025
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Here is my patch:

diff --git a/src/bin/cfgmgr/plugins/Makefile.am b/src/bin/cfgmgr/plugins/Makefile.am
index 529a4ed..e01d530 100644
--- a/src/bin/cfgmgr/plugins/Makefile.am
+++ b/src/bin/cfgmgr/plugins/Makefile.am
@@ -1,11 +1,15 @@
 SUBDIRS = tests
+
 EXTRA_DIST = README tsig_keys.py tsig_keys.spec
 EXTRA_DIST += logging.spec b10logging.py
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
-config_plugin_DATA = tsig_keys.py tsig_keys.spec
-config_plugin_DATA += b10logging.py logging.spec
+config_plugin_DATA = logging.spec tsig_keys.spec
+
+python_PYTHON = b10logging.py tsig_keys.py
+pythondir = $(config_plugindir)
 
+CLEANFILES = b10logging.pyc tsig_keys.pyc
 CLEANDIRS = __pycache__
 
 clean-local:

Which results in: "Byte-compiling python modules..." and "Byte-compiling python modules (optimized versions) ..."

That fixes problem of missing .pyc and .pyo files at installation time (to help packagers have same files at deinstallation time).

At first I thought the location was wrong since "share" is for "architecture-independent text files". But shane tells me they are machine independent. But they are python version dependent, so since it should be python3.1 identified that maybe keeping under normal python versioned site-packages is better. Is it okay to keep non-python modules (like our config_plugins) under normal site-packages location?

(So if moving under a pkgpyexecdir or pkgpythondir then the code using the plugins will also need to be patched.)

Subtickets

Change History (8)

comment:1 in reply to: ↑ description ; follow-up: Changed 8 years ago by vorner

Hello

Replying to jreed:

At first I thought the location was wrong since "share" is for "architecture-independent text files". But shane tells me they are machine independent. But they are python version dependent, so since it should be python3.1 identified that maybe keeping under normal python versioned site-packages is better. Is it okay to keep non-python modules (like our config_plugins) under normal site-packages location?

First, to correct something slightly. They are python, but not modules. Or, not modules in the usual sense, they, in theory, could be used as modules.

But if somebody accidentally used them as modules, he would be probably surprised. So I wouldn't like to have them somewhere an import could find them by itself.

Also, the plugin-loading machinery just goes through a directory and loads everything it finds there. So nothing else should get into that place also.

Otherwise, I don't really care about where they live, if the config manager can know it by itself at startup.

comment:2 in reply to: ↑ 1 Changed 8 years ago by jreed

Replying to vorner:

Also, the plugin-loading machinery just goes through a directory and loads everything it finds there. So nothing else should get into that place also.

Can you explain that from the following context?

freebsd8-32-1-on-virt1# ls -l /usr/local/share/bind10-devel/config_plugins/
total 22
-r--r--r--  1 root  wheel  5285 Sep 29 20:21 b10logging.py
-r--r--r--  1 root  wheel  2545 Sep 29 19:39 b10logging.pyc
-r--r--r--  1 root  wheel  2990 Sep 29 20:21 logging.spec
-r--r--r--  1 root  wheel  2097 Sep 29 20:21 tsig_keys.py
-r--r--r--  1 root  wheel  1407 Sep 29 19:39 tsig_keys.pyc
-r--r--r--  1 root  wheel   589 Sep 29 20:21 tsig_keys.spec

Note that these .pyc files were created at run-time and not installation time, so next time it runs does it attempt to load "everything" (so duplicating)?

Last edited 8 years ago by jreed (previous) (diff)

comment:3 Changed 8 years ago by vorner

No, not that much everything ;-). I meant *.py everything.

But if anybody installed some other python module into it (because they liked the place and thought that because it's site-packages, it is the correct place), it would try to load it and then crash, because the false „plugin“ wouldn't have a load function (or had and it would do something else).

comment:4 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Sprint-20111011

It seems like this ticket is being worked on, so I'm moving to the current sprint.

comment:5 Changed 8 years ago by jelte

  • Milestone changed from Sprint-20111011 to Sprint-20111025

comment:6 Changed 8 years ago by jreed

  • Owner set to UnAssigned
  • Status changed from new to reviewing

I plan to commit this patch with this commit message:

Use python_PYTHON for the python scripts. This results in
installing byte compiled version.

Removed files from EXTRA_DIST as they are now in dist by default.

Added CLEANFILES to clean up .pyc in build directory.
 
Note that I am keeping the installation directory the same.

--- a/src/bin/cfgmgr/plugins/Makefile.am
+++ b/src/bin/cfgmgr/plugins/Makefile.am
@@ -1,11 +1,14 @@
 SUBDIRS = tests
-EXTRA_DIST = README tsig_keys.py tsig_keys.spec
-EXTRA_DIST += logging.spec b10logging.py
+
+EXTRA_DIST = README logging.spec tsig_keys.spec
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
-config_plugin_DATA = tsig_keys.py tsig_keys.spec
-config_plugin_DATA += b10logging.py logging.spec
+config_plugin_DATA = logging.spec tsig_keys.spec
+
+python_PYTHON = b10logging.py tsig_keys.py
+pythondir = $(config_plugindir)
 
+CLEANFILES = b10logging.pyc tsig_keys.pyc
 CLEANDIRS = __pycache__
 
 clean-local:

comment:7 Changed 8 years ago by jreed

  • Resolution set to fixed
  • Status changed from reviewing to closed

commit aadf8f9

comment:8 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 2
Note: See TracTickets for help on using tickets.