Skip to content

Commit

Permalink
Add parenthesis around dependencies with extras (#1132)
Browse files Browse the repository at this point in the history
* Add parenthesis around dependencies with extras

`dependency` can be a valid spec with an `or` operation, and adding the
`and extra` to that statement will break it.

This PR always wraps the environment markers with parenthesis to prevent this,
and updates existing tests to expect the parenthesis.

* Formatted changes
  • Loading branch information
BryceStevenWilley authored Dec 13, 2023
1 parent e0449be commit 2741233
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 15 deletions.
9 changes: 6 additions & 3 deletions backend/src/hatchling/metadata/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ def construct_metadata_file_2_1(metadata: ProjectMetadata, extra_dependencies: t
metadata_file += f'Provides-Extra: {option}\n'
for dependency in dependencies:
if ';' in dependency:
metadata_file += f'Requires-Dist: {dependency} and extra == {option!r}\n'
dep_name, dep_env_marker = dependency.split(';', maxsplit=1)
metadata_file += f'Requires-Dist: {dep_name}; ({dep_env_marker.strip()}) and extra == {option!r}\n'
elif '@ ' in dependency:
metadata_file += f'Requires-Dist: {dependency} ; extra == {option!r}\n'
else:
Expand Down Expand Up @@ -228,7 +229,8 @@ def construct_metadata_file_2_2(metadata: ProjectMetadata, extra_dependencies: t
metadata_file += f'Provides-Extra: {option}\n'
for dependency in dependencies:
if ';' in dependency:
metadata_file += f'Requires-Dist: {dependency} and extra == {option!r}\n'
dep_name, dep_env_marker = dependency.split(';', maxsplit=1)
metadata_file += f'Requires-Dist: {dep_name}; ({dep_env_marker.strip()}) and extra == {option!r}\n'
elif '@ ' in dependency:
metadata_file += f'Requires-Dist: {dependency} ; extra == {option!r}\n'
else:
Expand Down Expand Up @@ -298,7 +300,8 @@ def construct_metadata_file_2_3(metadata: ProjectMetadata, extra_dependencies: t
metadata_file += f'Provides-Extra: {option}\n'
for dependency in dependencies:
if ';' in dependency:
metadata_file += f'Requires-Dist: {dependency} and extra == {option!r}\n'
dep_name, dep_env_marker = dependency.split(';', maxsplit=1)
metadata_file += f'Requires-Dist: {dep_name}; ({dep_env_marker.strip()}) and extra == {option!r}\n'
elif '@ ' in dependency:
metadata_file += f'Requires-Dist: {dependency} ; extra == {option!r}\n'
else:
Expand Down
54 changes: 42 additions & 12 deletions tests/backend/metadata/test_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,11 @@ def test_optional_dependencies(self, constructor, isolation, helpers):
Name: My.App
Version: 0.1.0
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
"""
)

Expand Down Expand Up @@ -743,11 +743,11 @@ def test_all(self, constructor, helpers, temp_dir):
Requires-Dist: bar==5
Requires-Dist: foo==1
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
Provides-Extra: feature3
Requires-Dist: baz@ file:///path/to/project ; extra == 'feature3'
Description-Content-Type: text/markdown
Expand Down Expand Up @@ -1062,11 +1062,41 @@ def test_optional_dependencies(self, constructor, isolation, helpers):
Name: My.App
Version: 0.1.0
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
"""
)

def test_optional_complex_dependencies(self, constructor, isolation, helpers):
metadata = ProjectMetadata(
str(isolation),
None,
{
'project': {
'name': 'My.App',
'version': '0.1.0',
'optional-dependencies': {
'feature2': ['foo==1; sys_platform == "win32" or python_version < "3"', 'bar==5'],
'feature1': ['foo==1', 'bar==5; python_version < "3"'],
},
}
},
)

assert constructor(metadata) == helpers.dedent(
"""
Metadata-Version: 2.2
Name: My.App
Version: 0.1.0
Provides-Extra: feature1
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; (sys_platform == 'win32' or python_version < '3') and extra == 'feature2'
"""
)

Expand Down Expand Up @@ -1165,11 +1195,11 @@ def test_all(self, constructor, helpers, temp_dir):
Requires-Dist: bar==5
Requires-Dist: foo==1
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
Provides-Extra: feature3
Requires-Dist: baz@ file:///path/to/project ; extra == 'feature3'
Description-Content-Type: text/markdown
Expand Down Expand Up @@ -1491,11 +1521,11 @@ def test_optional_dependencies(self, constructor, isolation, helpers):
Name: My.App
Version: 0.1.0
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
"""
)

Expand Down Expand Up @@ -1598,11 +1628,11 @@ def test_all(self, constructor, temp_dir, helpers):
Requires-Dist: bar==5
Requires-Dist: foo==1
Provides-Extra: feature1
Requires-Dist: bar==5; python_version < '3' and extra == 'feature1'
Requires-Dist: bar==5; (python_version < '3') and extra == 'feature1'
Requires-Dist: foo==1; extra == 'feature1'
Provides-Extra: feature2
Requires-Dist: bar==5; extra == 'feature2'
Requires-Dist: foo==1; python_version < '3' and extra == 'feature2'
Requires-Dist: foo==1; (python_version < '3') and extra == 'feature2'
Provides-Extra: feature3
Requires-Dist: baz@ file:///path/to/project ; extra == 'feature3'
Description-Content-Type: text/markdown
Expand Down

0 comments on commit 2741233

Please sign in to comment.