Commit e37aa992 authored by Łukasz Nowak's avatar Łukasz Nowak Committed by Xavier Thompson

Do not reprocess already extended files

extends can be interpreted as inheritance in OOP, but the original
behaviour was against what is commonly (always?) seen everywhere.

It is however good practice a file extends all files it needs directly
(and only them). Then if two files A & B (possibly unrelated) extends
the same third C, A was unable to overrides C values. It was even
error-prone because someone who don't use B yet could override C values
in A and later extending B would break A.

For some of our common use cases, this new algorithm is also 9x faster
(time to annotate: ~2.3s with -> ~.29s).

Other changes:
- ~/ is now expanded for non-url extends.
- An absolute (non-url) path is not longer treated like a local path
  if the base is a url.
- Better path/url normalization.

Rebase instructions:
- squash with "Chomp ../ from beginging of filenames"
- split and apply "Support ${:_profile_base_location_}." after

Sqaushed with

Chomp ../ from beginging of filenames.

In order to have as canonical as possible paths, chomp ../ from filenames and
recalculate base.
parent 16639592
...@@ -27,6 +27,11 @@ try: ...@@ -27,6 +27,11 @@ try:
except ImportError: except ImportError:
from collections import MutableMapping as DictMixin from collections import MutableMapping as DictMixin
try:
from urllib.parse import urljoin
except ImportError:
from urlparse import urljoin
import zc.buildout.configparser import zc.buildout.configparser
import copy import copy
import datetime import datetime
...@@ -207,7 +212,7 @@ class Buildout(DictMixin): ...@@ -207,7 +212,7 @@ class Buildout(DictMixin):
for (section, v) in itertools.groupby(sorted(cloptions), for (section, v) in itertools.groupby(sorted(cloptions),
lambda v: v[0]) lambda v: v[0])
) )
override = cloptions.get('buildout', {}).copy() override = cloptions.get('buildout', {})
# load user defaults, which override defaults # load user defaults, which override defaults
if user_defaults: if user_defaults:
...@@ -219,13 +224,12 @@ class Buildout(DictMixin): ...@@ -219,13 +224,12 @@ class Buildout(DictMixin):
user_config = os.path.join(buildout_home, 'default.cfg') user_config = os.path.join(buildout_home, 'default.cfg')
if os.path.exists(user_config): if os.path.exists(user_config):
_update(data, _open(os.path.dirname(user_config), user_config, _update(data, _open(os.path.dirname(user_config), user_config,
[], data['buildout'].copy(), override, data['buildout'], override))
set()))
# load configuration files # load configuration files
if config_file: if config_file:
_update(data, _open(os.path.dirname(config_file), config_file, [], _update(data, _open(os.path.dirname(config_file), config_file,
data['buildout'].copy(), override, set())) data['buildout'], override))
# apply command-line options # apply command-line options
_update(data, cloptions) _update(data, cloptions)
...@@ -1592,58 +1596,55 @@ def _default_globals(): ...@@ -1592,58 +1596,55 @@ def _default_globals():
return globals_defs return globals_defs
def _open(base, filename, seen, dl_options, override, downloaded): def _open(base, filename, dl_options, override, seen=None, processing=None):
"""Open a configuration file and return the result as a dictionary, """Open a configuration file and return the result as a dictionary,
Recursively open other files based on buildout options found. Recursively open other files based on buildout options found.
""" """
_update_section(dl_options, override)
_dl_options = _unannotate_section(dl_options.copy())
newest = bool_option(_dl_options, 'newest', 'false')
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
downloaded_filename = None
if _isurl(filename): if _isurl(filename):
downloaded_filename, is_temp = download(filename) download = True
fp = open(downloaded_filename) else:
base = filename[:filename.rfind('/')] download = _isurl(base)
elif _isurl(base): if download:
if os.path.isabs(filename): filename = urljoin(base + '/', filename)
fp = open(filename)
base = os.path.dirname(filename)
else: else:
filename = base + '/' + filename filename = os.path.realpath(os.path.join(
downloaded_filename, is_temp = download(filename) base, os.path.expanduser(filename)))
fp = open(downloaded_filename)
base = filename[:filename.rfind('/')] if seen:
if filename in seen:
if filename in processing:
raise zc.buildout.UserError("circular extends: %s" % filename)
return {}
seen.add(filename)
else: else:
filename = os.path.join(base, filename) seen = {filename}
fp = open(filename)
base = os.path.dirname(filename) is_temp = False
downloaded.add(filename) try:
if download:
if override is None:
_dl_options = dl_options
else:
_dl_options = _update_section(dl_options.copy(), override)
_unannotate_section(_dl_options)
downloaded_filename, is_temp = zc.buildout.download.Download(
_dl_options, cache=_dl_options.get('extends-cache'),
fallback=bool_option(_dl_options, 'newest', 'false'),
hash_name=True)(filename)
filename_for_logging = '%s (downloaded as %s)' % (
filename, downloaded_filename)
base = filename[:filename.rfind('/')]
else:
downloaded_filename = filename_for_logging = filename
base = os.path.dirname(filename)
if filename in seen: with open(downloaded_filename) as fp:
result = zc.buildout.configparser.parse(
fp, filename_for_logging, _default_globals)
finally:
if is_temp: if is_temp:
fp.close()
os.remove(downloaded_filename) os.remove(downloaded_filename)
raise zc.buildout.UserError("Recursive file include", seen, filename)
root_config_file = not seen
seen.append(filename)
filename_for_logging = filename
if downloaded_filename:
filename_for_logging = '%s (downloaded as %s)' % (
filename, downloaded_filename)
result = zc.buildout.configparser.parse(
fp, filename_for_logging, _default_globals)
fp.close()
if is_temp:
os.remove(downloaded_filename)
options = result.get('buildout', {}) options = result.get('buildout', {})
extends = options.pop('extends', None) extends = options.pop('extends', None)
...@@ -1652,21 +1653,22 @@ def _open(base, filename, seen, dl_options, override, downloaded): ...@@ -1652,21 +1653,22 @@ def _open(base, filename, seen, dl_options, override, downloaded):
'No-longer supported "extended-by" option found in %s.' % 'No-longer supported "extended-by" option found in %s.' %
filename) filename)
result = _annotate(result, filename) _annotate(result, filename)
if root_config_file and 'buildout' in result:
dl_options = _update_section(dl_options, result['buildout'])
if extends: if extends:
extends = extends.split() if processing:
eresult = _open(base, extends.pop(0), seen, dl_options, override, processing.append(filename)
downloaded) else:
for fname in extends: processing = [filename]
_update(eresult, _open(base, fname, seen, dl_options, override, # From now on, it won't change.
downloaded)) dl_options = _unannotate_section(_update_section(_update_section(
result = _update(eresult, result) dl_options.copy(), options), override))
override = None
seen.pop() eresult = {}
for extends in extends.split():
_update(eresult, _open(base, extends, dl_options, override,
seen, processing))
del processing[-1]
return _update(eresult, result)
return result return result
......
...@@ -457,12 +457,30 @@ used: ...@@ -457,12 +457,30 @@ used:
... extends = %sbase.cfg ... extends = %sbase.cfg
... bar = foo ... bar = foo
... """ % server_url) ... """ % server_url)
>>> write(server_data, 'baseC.cfg', """\
... [buildout]
... extends-cache = cache
... extends = baseB.cfg
... bar = foo
... """)
>>> write(server_data, 'baseD.cfg', """\
... [buildout]
... extends-cache = cache
... bar = foo
... """)
>>> mkdir(server_data, 'deeper')
>>> write(server_data, 'deeper', 'base.cfg', """\
... [buildout]
... extends-cache = cache
... extends = ../baseD.cfg
... bar = foo
... """)
>>> write('buildout.cfg', """\ >>> write('buildout.cfg', """\
... [buildout] ... [buildout]
... extends-cache = cache ... extends-cache = cache
... newest = true ... newest = true
... extends = %sbaseA.cfg %sbaseB.cfg ... extends = %sbaseA.cfg %sbaseB.cfg %sbaseC.cfg %sdeeper/base.cfg
... """ % (server_url, server_url)) ... """ % (server_url, server_url, server_url, server_url))
>>> print_(system(buildout + " -n")) >>> print_(system(buildout + " -n"))
Unused options for buildout: 'bar' 'foo'. Unused options for buildout: 'bar' 'foo'.
...@@ -480,6 +498,9 @@ a better solution would re-use the logging already done by the utility.) ...@@ -480,6 +498,9 @@ a better solution would re-use the logging already done by the utility.)
The URL http://localhost/baseA.cfg was downloaded. The URL http://localhost/baseA.cfg was downloaded.
The URL http://localhost/base.cfg was downloaded. The URL http://localhost/base.cfg was downloaded.
The URL http://localhost/baseB.cfg was downloaded. The URL http://localhost/baseB.cfg was downloaded.
The URL http://localhost/baseC.cfg was downloaded.
The URL http://localhost/deeper/base.cfg was downloaded.
The URL http://localhost/baseD.cfg was downloaded.
Not upgrading because not running a local buildout command. Not upgrading because not running a local buildout command.
Unused options for buildout: 'bar' 'foo'. Unused options for buildout: 'bar' 'foo'.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment