[omniORB] omniORBpy: __init__.py installed in site-packages

[Floris Bruynooghe]

Hi

It seems that __init__.py gets installed in the toplevel
site-packages. While generally not very harmful I'm sure it can
hardly be deemed as correct. The offending code seems to be in
python/COS/dir.mk, here a small patch which fixes this:

--- a/python/COS/dir.mk
+++ b/python/COS/dir.mk
@@ -78,8 +78,8 @@
EXPORTEDFILES += $(foreach f, $(filter CosNaming%, $(FILES)),
$(PYLIBROOT)/$(f))

# __init__.py to make it a package
-FILES += __init__.py
-INSTALLEDFILES += $(INSTALLPYTHONDIR)/__init__.py
+#FILES += __init__.py
+#INSTALLEDFILES += $(INSTALLPYTHONDIR)/__init__.py

# A .pth file to expose omniORB/COS to the global namespace
FILES += omniORB.pth


I'm not sure why that was there, am I disabling something important by
this change?

Regards
Floris

[Thomas Lockhart]

Post by Floris Bruynooghe
Hi
It seems that __init__.py gets installed in the toplevel
site-packages. While generally not very harmful I'm sure it can
hardly be deemed as correct. The offending code seems to be in
...
I'm not sure why that was there, am I disabling something important by
this change?

Isn't this required to do a clean install into an alternate location?
Breaking that installation is probably not a step forward.

This is not an issue for things like RPM installations since that file
can be omitted from the package.

Can the makefile be smart enough to look for an existing __init__.py
before installing its own? Or is there another python convention (and
code support) for how to install into a shared area?

- Tom

[Floris Bruynooghe]

Post by Thomas Lockhart

Post by Floris Bruynooghe
It seems that __init__.py gets installed in the toplevel
site-packages. ?While generally not very harmful I'm sure it can
hardly be deemed as correct. ?The offending code seems to be in
...
I'm not sure why that was there, am I disabling something important by
this change?

Isn't this required to do a clean install into an alternate location?
Breaking that installation is probably not a step forward.

I doubt it. omniorbpy needs to make it's own directories packages but
it can't ensure that the directory containing those packages ends up
on sys.path anyway. And turning the directory on sys.path into a
package itself just seems wrong.

Post by Thomas Lockhart
This is not an issue for things like RPM installations since that file can
be omitted from the package

Sure, I don't mind patching it on Debian's side (in fact we will in
the current release). I just don't understand yet why it was there in
the first place and am worried that it is (a small) error.


Regards
Floris

[Thomas Lockhart]

Post by Floris Bruynooghe

Post by Thomas Lockhart

Post by Floris Bruynooghe
It seems that __init__.py gets installed in the toplevel
site-packages. =C2=A0While generally not very harmful I'm sure it can
hardly be deemed as correct...

Isn't this required to do a clean install into an alternate location?
Breaking that installation is probably not a step forward.

I doubt it. omniorbpy needs to make it's own directories packages but
it can't ensure that the directory containing those packages ends up
on sys.path anyway. And turning the directory on sys.path into a
package itself just seems wrong.

OK, I see that my top-level directories on sys.path do not need an
__init__.py file to be recognized as the top-level of a python library
installation. Sorry, I should have checked that before responding. So I
don't see that is is necessary for omniORBpy (or omniORB; did you look
there too? There is some python code to support omniidl).

Duncan, does this look reasonable to apply to the main tree? Or should
we put this in as a patch for RPM building etc. for awhile first?

- Tom

[Floris Bruynooghe]

Post by Thomas Lockhart
OK, I see that my top-level directories on sys.path do not need an
__init__.py file to be recognized as the top-level of a python library
installation. Sorry, I should have checked that before responding. So I
don't see that is is necessary for omniORBpy (or omniORB; did you look there
too? There is some python code to support omniidl).

No, only omniORBpy seems to do this.


Floris

[Duncan Grisby]

Post by Thomas Lockhart

Post by Floris Bruynooghe
It seems that __init__.py gets installed in the toplevel
site-packages. =C2=A0While generally not very harmful I'm sure it can
hardly be deemed as correct...

[...]

Post by Thomas Lockhart
Duncan, does this look reasonable to apply to the main tree? Or should
we put this in as a patch for RPM building etc. for awhile first?

Well, the make rules in question were written by you in the first place!
To be honest I'd never noticed that the COS rules were installing an
__init__.py at the top level. I don't think there's any reason to keep
doing that so I've removed it.

Cheers,

Duncan.