Re: [gentoo-portage-dev] [PATCH] FEATURES=case-insensitive-fs for bug #524236
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 13/11/14 02:22, Zac Medico wrote: +if case-insensitive-fs in portage.settings.features: +FIND_EXTANT_CONFIGS = \ +FIND_EXTANT_CONFIGS.replace(-name '._cfg, -iname '._cfg) + Splitting inside the replace will look nicer following PEP indentation (as you won't need the '\'). +Use case\-insensitive file name comparisions when merging and unmerging +files. +.TP Maybe mention a) that most people can ignore this option, and b) who it's actually for. Kind of in the kernel option help style. In general I don't like this patch. It handles a bunch of cases separately by doing lower(), when I think instead it should be handled implicitly. The data should be in a structure such that it knows whether it is supposed to be upper or lowercase, and whatever's dealing with it should deal with it accordingly, rather than checking is this case insensitive? OK lowercase it before sending it wherever. But if you think this is the best way, I'm not going to stand in the way of this patch. - -- Alexander berna...@gentoo.org https://secure.plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlRkiB0ACgkQRtClrXBQc7UudAD/V1r4AR5zA54Xz+LHBVNt0bnn uQ9w+146L8WYK6pDN9gBAJxZbQREeOwxKSDjluZ1lq9XARf1rh/Eqzl58wIc8I4K =c1Em -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH] FEATURES=case-insensitive-fs for bug #524236
On 11/13/2014 02:29 AM, Alexander Berntsen wrote: On 13/11/14 02:22, Zac Medico wrote: +if case-insensitive-fs in portage.settings.features: +FIND_EXTANT_CONFIGS = \ +FIND_EXTANT_CONFIGS.replace(-name '._cfg, -iname '._cfg) + Splitting inside the replace will look nicer following PEP indentation (as you won't need the '\'). Okay, I'll re-format it as you've suggested. +Use case\-insensitive file name comparisions when merging and unmerging +files. +.TP Maybe mention a) that most people can ignore this option, and b) who it's actually for. Kind of in the kernel option help style. Okay, I'll add something about it only being needed for case-insensitive file systems, which are usually not used. In general I don't like this patch. It handles a bunch of cases separately by doing lower(), when I think instead it should be handled implicitly. The data should be in a structure such that it knows whether it is supposed to be upper or lowercase, and whatever's dealing with it should deal with it accordingly, rather than checking is this case insensitive? OK lowercase it before sending it wherever. Aside from the ConfigProtect constructor, which has a new case_insensitive keyword parameter, all affected methods handle case transformations implicitly, as far as API consumers are concerned. However, we could improve efficiency for some usage patterns by providing an alternative to dblink.getcontents that is oriented toward case-insensitive handling. For example, every single dblink._match_contents call currently has to transform all names to lowercase, and generate a reverse mapping from lowercase back to preserved case. The dblink._match_contents method would be more efficient if we created an alternative to dblink.getcontents that handled the transformations and cached the results. I intentionally did not implement this optimization yet, since it's probably better to do it in a separate patch, rather than complicate the current patch. But if you think this is the best way, I'm not going to stand in the way of this patch. As discussed above, think the current approach is pretty reasonable, though I would like to optimize it with a separate patch. -- Thanks, Zac
[gentoo-portage-dev] [PATCH] fs_template._ensure_dirs: handle EEXIST (529120)
There was a race inside fs_template._ensure_dirs which could cause it to raise EEXIST if a concurrent process created the directory after os.path.exists returned False. Fix it by using the util.ensure_dirs function, which already handles EEXIST. X-Gentoo-Bug: 529120 X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=529120 --- pym/portage/cache/fs_template.py | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/pym/portage/cache/fs_template.py b/pym/portage/cache/fs_template.py index de4fe4b..fa44abc 100644 --- a/pym/portage/cache/fs_template.py +++ b/pym/portage/cache/fs_template.py @@ -10,7 +10,7 @@ from portage import os from portage.proxy.lazyimport import lazyimport lazyimport(globals(), 'portage.exception:PortageException', - 'portage.util:apply_permissions', + 'portage.util:apply_permissions,ensure_dirs', ) del lazyimport @@ -61,20 +61,15 @@ class FsBased(template.database): for dir in path.lstrip(os.path.sep).rstrip(os.path.sep).split(os.path.sep): base = os.path.join(base,dir) - if not os.path.exists(base): - if self._perms != -1: - um = os.umask(0) - try: - perms = self._perms - if perms == -1: - perms = 0 - perms |= 0o755 - os.mkdir(base, perms) - if self._gid != -1: - os.chown(base, -1, self._gid) - finally: - if self._perms != -1: - os.umask(um) + if ensure_dirs(base): + # We only call apply_permissions if ensure_dirs created + # a new directory, so as not to interfere with + # permissions of existing directories. + mode = self._perms + if mode == -1: + mode = 0 + mode |= 0o755 + apply_permissions(base, mode=mode, gid=self._gid) def _prune_empty_dirs(self): all_dirs = [] -- 2.0.4
[gentoo-portage-dev] [PATCH] portageq: fix eroot parameter (bug #529200)
The portageq eroot parameter has been broken since commit c9f6aa9f0151adb3c86706eaef1914cdbdcf2b6d, due to premature instantiation of portage.settings (before the ROOT variable was set). Premature access to the portage.settings attribute must be avoided by using other available means to determine the EPREFIX. Fixes: c9f6aa9f0151 (Add cross-prefix support) X-Gentoo-Bug: 529200 X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=529200 --- bin/portageq| 9 - pym/portage/tests/emerge/test_simple.py | 10 -- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/bin/portageq b/bin/portageq index 009f116..ef565d1 100755 --- a/bin/portageq +++ b/bin/portageq @@ -1392,7 +1392,14 @@ def main(argv): sys.stderr.write(Run portageq with --help for info\n) sys.stderr.flush() sys.exit(os.EX_USAGE) - eprefix = portage.settings[EPREFIX] + # Calculate EPREFIX and ROOT that will be used to construct + # portage.settings later. It's tempting to use + # portage.settings[EPREFIX] here, but that would force + # instantiation of portage.settings, which we don't want to do + # until after we've calculated ROOT (see bug #529200). + eprefix = os.environ.get(EPREFIX, portage.const.EPREFIX) + if eprefix: + eprefix = portage.util.normalize_path(eprefix) eroot = portage.util.normalize_path(argv[2]) if eprefix: diff --git a/pym/portage/tests/emerge/test_simple.py b/pym/portage/tests/emerge/test_simple.py index 6c20a07..0101362 100644 --- a/pym/portage/tests/emerge/test_simple.py +++ b/pym/portage/tests/emerge/test_simple.py @@ -217,6 +217,8 @@ pkg_preinst() { self.assertFalse(test_ebuild is None) cross_prefix = os.path.join(eprefix, cross_prefix) + cross_root = os.path.join(eprefix, cross_root) + cross_eroot = os.path.join(cross_root, eprefix.lstrip(os.sep)) test_commands = ( env_update_cmd, @@ -318,6 +320,10 @@ pkg_preinst() { portageq_cmd + (has_version, cross_prefix, dev-libs/A), ({EPREFIX : cross_prefix},) + \ portageq_cmd + (has_version, cross_prefix, dev-libs/B), + + # Test ROOT support + ({ROOT: cross_root},) + emerge_cmd + (dev-libs/B,), + portageq_cmd + (has_version, cross_eroot, dev-libs/B), ) distdir = playground.distdir @@ -372,8 +378,8 @@ pkg_preinst() { os.environ[__PORTAGE_TEST_HARDLINK_LOCKS] updates_dir = os.path.join(test_repo_location, profiles, updates) - dirs = [cachedir, cachedir_pregen, cross_prefix, distdir, fake_bin, - portage_tmpdir, updates_dir, + dirs = [cachedir, cachedir_pregen, cross_eroot, cross_prefix, + distdir, fake_bin, portage_tmpdir, updates_dir, user_config_dir, var_cache_edb] etc_symlinks = (dispatch-conf.conf, etc-update.conf) # Override things that may be unavailable, or may have portability -- 2.0.4