On Tue, Jul 13, 2010 at 14:46, Hanno Schlichting <ha...@hannosch.eu> wrote: > On Tue, Jul 13, 2010 at 7:05 PM, Leonardo Rochael Almeida > <leoroch...@gmail.com> wrote: >> And I'm not disagreeing with the policy, but it can be argued that >> depending on the location of *data* files inside the Zope2 package is >> not necessarily relying on "guarantees on internals". > > I'd call anything that relies on a specific file system layout to be > internals. We only make guarantees on the semantics of the Python > import system, not how packages and modules are stitched together in > distributions and end up on the file system.
Considering that the Zope source code has a dozen examples of App.ImageFile.ImageFile being used without a prefix (ex. "OFS.misc_"), I think third party developers should be excused to think they could follow the lead and expect their packages not to break during a stable series. I still think we should give them at least the 2.12 series to stop using ImageFile "incorrectly" without breaking their code, but I won't belabor the point any longer. >> Obviously. Still, App.ImageFile.ImageFile (and any other Zope2 APIs >> that do rely on software_home) should give a clear warning when >> software_home is assumed, since it is the one assuming it. The >> exception raised by the ImageFile call now gives no clue on what >> actually went wrong and what a developer should do about it. > > I agree with that. I thought the call to software home was being made > inside ZMySQLDA and not in Zope 2 code. The attached patch adds such a warning, and also reveals where Zope2 commits the same sins. If there are no objections I'll commit it to 2.12 and trunk. Finally, I'd like to ask that, when major changes land in a stable series (like the spinning off of a whole package), that we allow at least a week or two to pass before making a release, so people who have test runners for their apps running against a stable repository branch have time to adapt to the changes Cheers, Leo
Index: src/App/config.py =================================================================== --- src/App/config.py (revisão 114644) +++ src/App/config.py (cópia de trabalho) @@ -36,7 +36,7 @@ def setConfiguration(cfg): """Set the global configuration object. - Legacy sources of common configuraiton values are updated to + Legacy sources of common configuration values are updated to reflect the new configuration; this may be removed in some future version. """ Index: src/App/tests/testImageFile.py =================================================================== --- src/App/tests/testImageFile.py (revisão 0) +++ src/App/tests/testImageFile.py (revisão 0) @@ -0,0 +1,28 @@ +import unittest +import App +from Testing.ZopeTestCase.warnhook import WarningsHook + + +class TestImageFile(unittest.TestCase): + + def setUp(self): + # ugly: need to save the old App.config configuration value since + # ImageFile might read it and trigger setting it to the default value + self.oldcfg = App.config._config + self.warningshook = WarningsHook() + self.warningshook.install() + + def tearDown(self): + self.warningshook.uninstall() + # ugly: need to restore configuration, or lack thereof + App.config._config = self.oldcfg + + def test_warn_on_software_home_default(self): + App.ImageFile.ImageFile('App/www/zopelogo.jpg') + self.assertEquals(self.warningshook.warnings.pop()[0], + App.ImageFile.NON_PREFIX_WARNING) + +def test_suite(): + return unittest.TestSuite(( + unittest.makeSuite(TestImageFile), + )) Index: src/App/ImageFile.py =================================================================== --- src/App/ImageFile.py (revisão 114644) +++ src/App/ImageFile.py (cópia de trabalho) @@ -18,6 +18,7 @@ import os.path import stat import time +import warnings from AccessControl.SecurityInfo import ClassSecurityInfo from Acquisition import Explicit @@ -34,6 +35,13 @@ os.path.join(os.path.dirname(Zope2.__file__), os.path.pardir) ) +NON_PREFIX_WARNING = ('Assuming image location to be present in the Zope2 ' + 'distribution. This is deprecated and might lead to ' + 'broken code if the directory in question is moved ' + 'to another distribution. Please provide either an ' + 'absolute file system path or a prefix. Support for ' + 'relative filenames without a prefix might be ' + 'dropped in a future Zope2 release.') class ImageFile(Explicit): """Image objects stored in external files.""" @@ -43,9 +51,12 @@ def __init__(self, path, _prefix=None): import Globals # for data if _prefix is None: - _prefix=getattr(getConfiguration(), 'softwarehome', PREFIX) + _prefix=getattr(getConfiguration(), 'softwarehome', None) or PREFIX + if not os.path.isabs(path): + warnings.warn(NON_PREFIX_WARNING, UserWarning, 2 ) elif type(_prefix) is not type(''): _prefix=package_home(_prefix) + # _prefix is ignored if path is absolute path = os.path.join(_prefix, path) self.path=path if Globals.DevelopmentMode:
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )