Hi,
I updated my pacth following Thomas recommendations (Attached).
I merged the 2 patches because it was connected and it was good to make
it cleaner.
Feel free to review (and apply/branch if ok),
Regards,
Rafael Monnerat
PS.: My english is not so good, so few free to improve my text on doctest :)
On 14-02-2011 04:54, Thomas Lotze wrote:
Rafael Monnerat wrote:
I posted the bugs (I think are 2 diferent) into launchpad with my patches
which I made on friday:
Thank you.
- https://bugs.launchpad.net/zc.buildout/+bug/717730
As far as the cache issue is concerned, this patch looks fine by me, but
of course, there should be a test for the changed behaviour.
OTOH, the patch reminds me of a different issue which I've been running
into with buildout a couple of times now: there should be an API somewhere
in zc.buildout that helps resolve string values from config files into
boolean values. I'll remember to bring this up as a separate issue.
- https://bugs.launchpad.net/zc.buildout/+bug/717802
* A test for the changed behaviour would be in order in this case as well.
* A set might be a more appropriate data structure than a list for
collecting the file names seen.
* The empty set of file names should be passed into the first call to
_open to avoid a mutable object as the default value of downloaded_list.
* The whole logic of the nested if-else blocks might be refactored to get
rid of code duplication (which has been there all along and becomes even
more apparent by your patch). Maybe this also makes the appropriate
fallback value known at the time of instantiating the download utility.
Index: src/zc/buildout/buildout.py
===================================================================
--- src/zc/buildout/buildout.py (revisão 120291)
+++ src/zc/buildout/buildout.py (cópia de trabalho)
@@ -193,12 +193,12 @@
'.buildout', 'default.cfg')
if os.path.exists(user_config):
_update(data, _open(os.path.dirname(user_config), user_config,
- [], data['buildout'].copy(), override))
+ [], data['buildout'].copy(), override, set()))
# load configuration files
if config_file:
_update(data, _open(os.path.dirname(config_file), config_file, [],
- data['buildout'].copy(), override))
+ data['buildout'].copy(), override, set()))
# apply command-line options
_update(data, cloptions)
@@ -1387,17 +1387,19 @@
for option, value in items:
_save_option(option, value, f)
-def _open(base, filename, seen, dl_options, override):
+def _open(base, filename, seen, dl_options, override, downloaded):
"""Open a configuration file and return the result as a dictionary,
Recursively open other files based on buildout options found.
"""
_update_section(dl_options, override)
_dl_options = _unannotate_section(dl_options.copy())
+ newest = _convert_bool('newest', _dl_options.get('newest'))
+ fallback = newest and not (filename in downloaded)
+ download = zc.buildout.download.Download(
+ _dl_options, cache=_dl_options.get('extends-cache'),
+ fallback=fallback, hash_name=True)
is_temp = False
- download = zc.buildout.download.Download(
- _dl_options, cache=_dl_options.get('extends-cache'), fallback=True,
- hash_name=True)
if _isurl(filename):
path, is_temp = download(filename)
fp = open(path)
@@ -1416,6 +1418,7 @@
fp = open(filename)
base = os.path.dirname(filename)
+ downloaded.add(filename)
if filename in seen:
if is_temp:
fp.close()
@@ -1449,9 +1452,11 @@
if extends:
extends = extends.split()
- eresult = _open(base, extends.pop(0), seen, dl_options, override)
+ eresult = _open(base, extends.pop(0), seen, dl_options, override,
+ downloaded)
for fname in extends:
- _update(eresult, _open(base, fname, seen, dl_options, override))
+ _update(eresult, _open(base, fname, seen, dl_options, override,
+ downloaded))
result = _update(eresult, result)
if extended_by:
@@ -1460,8 +1465,8 @@
)
for fname in extended_by.split():
result = _update(result,
- _open(base, fname, seen, dl_options, override))
-
+ _open(base, fname, seen, dl_options, override,
+ downloaded))
seen.pop()
return result
Index: src/zc/buildout/extends-cache.txt
===================================================================
--- src/zc/buildout/extends-cache.txt (revisão 120291)
+++ src/zc/buildout/extends-cache.txt (cópia de trabalho)
@@ -386,7 +386,100 @@
An internal error occurred ...
ValueError: install_from_cache set to true with no download cache
+>>> rmdir('home', '.buildout')
+Newest and non-newest behaviour for extends-cache
+-------------------------------------------------------
+
+Newest has a diferent behaviour them offline mode, it can download new files
+if those ones are not present yet. If we run buildout in newest mode
+(newest = true), the configuration files are downloaded (or updated) always,
+but if you use non-newest (newest = false), the files already present into
+extends-cache will not be downloaded again or updated.
+
+>>> mkdir("cache")
+>>> write(server_data, 'base.cfg', """\
+... [buildout]
+... parts =
+... """)
+>>> write('buildout.cfg', """\
+... [buildout]
+... extends-cache = cache
+... extends = %sbase.cfg
+... """ % server_url)
+>>> print system(buildout)
+>>> ls('cache')
+- 5aedc98d7e769290a29d654a591a3a45
+>>> cat('cache', os.listdir(cache)[0])
+[buildout]
+parts =
+
+Now let's update remote base.cfg.
+
+>>> write(server_data, 'base.cfg', """\
+... [buildout]
+... parts =
+... foo = bar
+... """)
+>>> print system(buildout + " -n")
+Unused options for buildout: 'foo'.
+>>> cat('cache', os.listdir(cache)[0])
+[buildout]
+parts =
+foo = bar
+
+Update back the base.cfg, and try non-newest.
+
+>>> write(server_data, 'base.cfg', """\
+... [buildout]
+... parts =
+... """)
+>>> print system(buildout + " -N")
+Unused options for buildout: 'foo'.
+>>> cat('cache', os.listdir(cache)[0])
+[buildout]
+parts =
+foo = bar
+
+Now we have to guarantee the download happens only once per url, by extending
+twice the same configuraton file:
+
+>>> write(server_data, 'baseA.cfg', """\
+... [buildout]
+... extends = %sbase.cfg
+... foo = bar
+... """ % server_url)
+>>> write(server_data, 'baseB.cfg', """\
+... [buildout]
+... extends-cache = cache
+... extends = %sbase.cfg
+... bar = foo
+... """ % server_url)
+>>> write('buildout.cfg', """\
+... [buildout]
+... extends-cache = cache
+... newest = true
+... extends = %sbaseA.cfg %sbaseB.cfg
+... """ % (server_url, server_url))
+>>> print system(buildout + " -n")
+Unused options for buildout: 'bar' 'foo'.
+
+Wrapper the Buildout download API to produce readable output for the test.
+
+>>> import zc.buildout
+>>> zc.buildout.download.Download.old_call = zc.buildout.download.Download.__call__
+>>> def wrapper_call(self, url, md5sum=None, path=None):
+... print "The URL %s was downloaded (%s)" % (url , self.fallback)
+... return self.old_call(url, md5sum, path)
+>>> zc.buildout.download.Download.__call__ = wrapper_call
+>>> zc.buildout.buildout.main([])
+The URL http://localhost/baseA.cfg was downloaded (True)
+The URL http://localhost/base.cfg was downloaded (True)
+The URL http://localhost/baseB.cfg was downloaded (True)
+The URL http://localhost/base.cfg was downloaded (False)
+Unused options for buildout: 'bar' 'foo'.
+
+
Clean up
--------
_______________________________________________
Distutils-SIG maillist - [email protected]
http://mail.python.org/mailman/listinfo/distutils-sig
