From c8c29e5ce18b6d4f50f5cffb02993e64a41ae0b0 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 29 Mar 2024 17:17:21 +0100 Subject: [PATCH] Preserve the case of package names in versions2constraints and friends. Fixes https://github.com/plone/plone.releaser/issues/65 --- news/65.bugfix | 2 + plone/releaser/base.py | 19 +++++++-- plone/releaser/buildout.py | 28 ++++++------- plone/releaser/pip.py | 17 ++++---- plone/releaser/tests/test_buildout.py | 39 +++++++++---------- plone/releaser/tests/test_pip.py | 33 ++++++++-------- .../tests/test_versions2constraints.py | 4 +- 7 files changed, 75 insertions(+), 67 deletions(-) create mode 100644 news/65.bugfix diff --git a/news/65.bugfix b/news/65.bugfix new file mode 100644 index 0000000..513252e --- /dev/null +++ b/news/65.bugfix @@ -0,0 +1,2 @@ +Preserve the case of package names in versions2constraints and friends. +[maurits] diff --git a/plone/releaser/base.py b/plone/releaser/base.py index 536710f..a401a2b 100644 --- a/plone/releaser/base.py +++ b/plone/releaser/base.py @@ -12,16 +12,27 @@ def __init__(self, file_location): def data(self): raise NotImplementedError + @property + def lowerkeys(self): + # Map from lower case key to actual key in the data. + return {key.lower(): key for key in self.data} + def __iter__(self): return self.data.__iter__() def __contains__(self, package_name): - return package_name.lower() in self.data + return package_name.lower() in self.lowerkeys def __getitem__(self, package_name): - if package_name in self: - return self.data.get(package_name.lower()) - raise KeyError + # self.data may be a defaultdict, so we cannot use + # 'return self.data[package_name]' + if package_name not in self: + raise KeyError(package_name) + # The package_name is in the data, but the case might differ. + if package_name in self.data: + return self.data[package_name] + actual_key = self.lowerkeys[package_name.lower()] + return self.data[actual_key] def __setitem__(self, package_name, value): raise NotImplementedError diff --git a/plone/releaser/buildout.py b/plone/releaser/buildout.py index 5dc5aeb..2b42a61 100644 --- a/plone/releaser/buildout.py +++ b/plone/releaser/buildout.py @@ -82,10 +82,8 @@ def config(self): # For versions.cfg we had strict=False, for the others not. # Let's use it always. config = ConfigParser(interpolation=ExtendedInterpolation(), strict=False) - # In SourcesFile we had this: - # config.optionxform = str - # This seems to make everything lowercase, and makes tests fail. - # TODO: we could use it if we choose. + # Preserve the case instead of the default lowercase transform: + config.optionxform = str with self.path.open() as f: config.read_file(f) # Especially in sources.cfg we may need to define a few extra variables @@ -103,7 +101,8 @@ def raw_config(self): # Read the same data, but without interpolation. # So keep a url like '${settings:plone}/package.git' config = ConfigParser(strict=False) - # config.optionxform = str + # Preserve the case instead of the default lowercase transform: + config.optionxform = str with self.path.open() as f: config.read_file(f) return config @@ -161,7 +160,7 @@ def data(self): for section in self.config.sections(): if section == "versions": for package, version in self.config[section].items(): - # Note: the package names are lower case. + # Note: the package names used to be lower case, but not anymore. versions[package][""] = version if not self.with_markers: continue @@ -298,7 +297,7 @@ def pins_to_pip(self): An option would be to always do this for Buildout as well. Or have a command to normalize a buildout file, with this and other - small changes like making all package named lower case. + small changes. """ new_data = {} for package, version in self.data.items(): @@ -353,7 +352,7 @@ def data(self): # I don't think we need to support [sources:marker]. for name, value in self.config["sources"].items(): source = Source.create_from_string(value) - sources_dict[name.lower()] = source + sources_dict[name] = source return sources_dict @property @@ -362,7 +361,7 @@ def raw_data(self): # I don't think we need to support [sources:marker]. for name, value in self.raw_config["sources"].items(): source = Source.create_from_string(value) - sources_dict[name.lower()] = source + sources_dict[name] = source return sources_dict def __setitem__(self, package_name, value): @@ -407,12 +406,13 @@ def always_checkout(self): def data(self): # I don't think we need to support [buildout:marker]. checkouts = self.config.get("buildout", "auto-checkout") - # Map from lower case to actual case, so we can find the package. + # In this case a set or list would be fine, but in all other + # cases the data is a dictionary, so let's use that. mapping = {} for package in checkouts.splitlines(): if not package: continue - mapping[package.lower()] = package + mapping[package] = True return mapping def __setitem__(self, package_name, enabled=True): @@ -448,11 +448,7 @@ def rewrite(self): if self.always_checkout: contents.append(f"always-checkout = {self.always_checkout}"), contents.append("auto-checkout =") - # self.values has the original case. - # We could iterate over 'self' to get lowercase, - # which is what we get in most other places. - # But for now let's use the info we have. - for package in self.values(): + for package in self: contents.append(f" {package}") contents.append("") new_contents = "\n".join(contents) diff --git a/plone/releaser/pip.py b/plone/releaser/pip.py index 0dda487..67baad0 100644 --- a/plone/releaser/pip.py +++ b/plone/releaser/pip.py @@ -60,7 +60,7 @@ def data(self): if "==" not in line: # We might want to support e.g. '>=', but for now keep it simple. continue - package = line.split("==")[0].strip().lower() + package = line.split("==")[0].strip() version = line.split("==", 1)[1].strip() # The line could also contain environment markers like this: # "; python_version >= '3.0'" @@ -165,8 +165,7 @@ def data(self): for package in self.config.sections(): use = to_bool(self.config[package].get("use", self.default_use)) if use: - # Map from lower case to actual case, so we can find the package. - checkouts[package.lower()] = package + checkouts[package] = True return checkouts @property @@ -174,10 +173,14 @@ def sections(self): # If we want to use a package, we must first know that it exists. sections = {} for package in self.config.sections(): - # Map from lower case to actual case, so we can find the package. - sections[package.lower()] = package + sections[package] = True return sections + @property + def lowerkeys_section(self): + # Map from lower case key to actual key in the sections. + return {key.lower(): key for key in self.sections} + def __setitem__(self, package_name, enabled=True): """Enable or disable a checkout. @@ -193,7 +196,7 @@ def __setitem__(self, package_name, enabled=True): So if the package we want to enable is not defined, meaning it has no section, then we should fail loudly. """ - stored_package_name = self.sections.get(package_name.lower()) + stored_package_name = self.lowerkeys_section.get(package_name.lower()) if not stored_package_name: raise KeyError( f"{self.file_location}: There is no definition for {package_name}" @@ -267,7 +270,7 @@ def rewrite(self): for key, value in self.config["settings"].items(): contents.append(f"{key} = {value}") - for package in self.sections.values(): + for package in self.sections: contents.append("") contents.append(f"[{package}]") for key, value in self.config[package].items(): diff --git a/plone/releaser/tests/test_buildout.py b/plone/releaser/tests/test_buildout.py index fcf089c..95e28a2 100644 --- a/plone/releaser/tests/test_buildout.py +++ b/plone/releaser/tests/test_buildout.py @@ -21,10 +21,11 @@ def test_checkouts_file_data(): cf = CheckoutsFile(CHECKOUTS_FILE) - # The data maps lower case to actual case. + # The data used to map lower case to actual case, + # but now actual case to True. assert cf.data == { - "camelcase": "CamelCase", - "package": "package", + "CamelCase": True, + "package": True, } @@ -40,12 +41,11 @@ def test_checkouts_file_contains(): def test_checkouts_file_get(): cf = CheckoutsFile(CHECKOUTS_FILE) - # The data maps lower case to actual case. - assert cf["package"] == "package" - assert cf.get("package") == "package" - assert cf["camelcase"] == "CamelCase" - assert cf["CAMELCASE"] == "CamelCase" - assert cf["CamelCase"] == "CamelCase" + assert cf["package"] is True + assert cf.get("package") is True + assert cf["camelcase"] is True + assert cf["CAMELCASE"] is True + assert cf["CamelCase"] is True with pytest.raises(KeyError): cf["nope"] @@ -60,7 +60,7 @@ def test_checkouts_file_add(tmp_path): # Let's read it fresh, for good measure. cf = CheckoutsFile(copy_path) assert "Extra" in cf - assert cf.get("extra") == "Extra" + assert cf.get("extra") is True def test_checkouts_file_remove(tmp_path): @@ -146,8 +146,7 @@ def test_source_docs(): def test_sources_file_data(): sf = SourcesFile(SOURCES_FILE) - # Note that the keys are lowercase. - assert sorted(sf.data.keys()) == ["docs", "plone", "plone.alterego", "plone.base"] + assert sorted(sf.data.keys()) == ["Plone", "docs", "plone.alterego", "plone.base"] def test_sources_file_contains(): @@ -220,7 +219,7 @@ def test_sources_file_rewrite(tmp_path): [sources] docs = git ${remotes:plone}/documentation.git branch=6.0 path=${buildout:docs-directory} egg=false -plone = git ${remotes:plone}/Plone.git pushurl=${remotes:plone_push}/Plone.git branch=6.0.x +Plone = git ${remotes:plone}/Plone.git pushurl=${remotes:plone_push}/Plone.git branch=6.0.x plone.alterego = git ${remotes:plone}/plone.alterego.git branch=master plone.base = git ${remotes:plone}/plone.base.git branch=main """ @@ -229,15 +228,14 @@ def test_sources_file_rewrite(tmp_path): def test_versions_file_versions(): vf = VersionsFile(VERSIONS_FILE) - # All versions are reported lowercased. assert vf.data == { "annotated": "1.0", - "camelcase": "1.0", + "CamelCase": "1.0", "duplicate": "1.0", "lowercase": "1.0", "package": "1.0", "pyspecific": "1.0", - "uppercase": "1.0", + "UPPERCASE": "1.0", } @@ -275,13 +273,13 @@ def test_versions_file_versions_with_markers(): # All versions are reported lowercased. assert vf.data == { "annotated": "1.0", - "camelcase": "1.0", + "CamelCase": "1.0", "duplicate": "1.0", "lowercase": "1.0", "onepython": {"python312": "2.1"}, "package": "1.0", "pyspecific": {"": "1.0", "python312": "2.0"}, - "uppercase": "1.0", + "UPPERCASE": "1.0", } @@ -452,7 +450,6 @@ def test_versions_file_rewrite(tmp_path): # - the extends line is on a separate line # - all comments are removed # - the duplicate is removed - # - all package names are lowercased assert ( copy_path.read_text() == """[buildout] @@ -461,12 +458,12 @@ def test_versions_file_rewrite(tmp_path): [versions] annotated = 1.0 -camelcase = 1.0 +CamelCase = 1.0 duplicate = 1.0 lowercase = 1.0 package = 1.0 pyspecific = 1.0 -uppercase = 1.0 +UPPERCASE = 1.0 """ ) diff --git a/plone/releaser/tests/test_pip.py b/plone/releaser/tests/test_pip.py index 78116e6..7386364 100644 --- a/plone/releaser/tests/test_pip.py +++ b/plone/releaser/tests/test_pip.py @@ -17,10 +17,11 @@ def test_mxdev_file_data(): mf = IniFile(MXDEV_FILE) - # The data maps lower case to actual case. + # The data used to map lower case to actual case, + # but now actual case to True. assert mf.data == { - "camelcase": "CamelCase", - "package": "package", + "CamelCase": True, + "package": True, } @@ -36,12 +37,11 @@ def test_mxdev_file_contains(): def test_mxdev_file_get(): mf = IniFile(MXDEV_FILE) - # The data maps lower case to actual case. - assert mf["package"] == "package" - assert mf.get("package") == "package" - assert mf["camelcase"] == "CamelCase" - assert mf["CAMELCASE"] == "CamelCase" - assert mf["CamelCase"] == "CamelCase" + assert mf["package"] is True + assert mf.get("package") is True + assert mf["camelcase"] is True + assert mf["CAMELCASE"] is True + assert mf["CamelCase"] is True with pytest.raises(KeyError): mf["unused"] assert mf.get("unused") is None @@ -57,7 +57,7 @@ def test_mxdev_file_add_known(tmp_path): # Let's read it fresh, for good measure. mf = IniFile(copy_path) assert "unused" in mf - assert mf["unused"] == "unused" + assert mf["unused"] is True def test_mxdev_file_add_unknown(tmp_path): @@ -137,12 +137,12 @@ def test_constraints_file_constraints(): # All constraints are reported lowercased. assert cf.data == { "annotated": "1.0", - "camelcase": "1.0", + "CamelCase": "1.0", "duplicate": "1.0", "lowercase": "1.0", "package": "1.0", "pyspecific": "1.0", - "uppercase": "1.0", + "UPPERCASE": "1.0", } @@ -276,13 +276,13 @@ def test_constraints_file_constraints_with_markers(): # All constraints are reported lowercased. assert cf.data == { "annotated": "1.0", - "camelcase": "1.0", + "CamelCase": "1.0", "duplicate": "1.0", "lowercase": "1.0", "onepython": {'python_version=="3.12"': "2.1"}, "package": "1.0", "pyspecific": {"": "1.0", 'python_version=="3.12"': "2.0"}, - "uppercase": "1.0", + "UPPERCASE": "1.0", } @@ -300,17 +300,16 @@ def test_constraints_file_rewrite(tmp_path): # - the extends line is on a separate line # - all comments are removed # - the duplicate is removed - # - all package names are lowercased assert ( copy_path.read_text() == """-c https://zopefoundation.github.io/Zope/releases/5.8.3/constraints.txt annotated==1.0 -camelcase==1.0 +CamelCase==1.0 duplicate==1.0 lowercase==1.0 package==1.0 pyspecific==1.0 -uppercase==1.0 +UPPERCASE==1.0 """ ) diff --git a/plone/releaser/tests/test_versions2constraints.py b/plone/releaser/tests/test_versions2constraints.py index 2e19522..42dc77c 100644 --- a/plone/releaser/tests/test_versions2constraints.py +++ b/plone/releaser/tests/test_versions2constraints.py @@ -28,13 +28,13 @@ def test_versions2constraints(tmp_path): constraints_file.read_text() == """-c https://zopefoundation.github.io/Zope/releases/5.8.3/constraints.txt annotated==1.0 -camelcase==1.0 +CamelCase==1.0 duplicate==1.0 lowercase==1.0 package==1.0 pyspecific==1.0 pyspecific==2.0; python_version == "3.12" -uppercase==1.0 +UPPERCASE==1.0 onepython==2.1; python_version == "3.12" """ )