`requirements/updater.sh` produces requirement comments pointing to incorrect sources

webknjaz commented 3 weeks ago

Please confirm the following

[X] I agree to follow this project's code of conduct.
[X] I have checked the current issues for duplicates.
[X] I understand that AWX is open source software provided for free and that I might not receive a timely response.
[X] I am NOT reporting a (potential) security vulnerability. (These should be emailed to security@ansible.com instead.)

Bug Summary

The updater script post-processes the pip-compile output in a way that re-assigns the source comments to incorrect package specifiers. This is because sed only removes the lines containing PEP 508 specifiers, but not the comments associated with them.

AWX version

devel

Select the relevant components

[ ] UI
[ ] UI (tech preview)
[ ] API
[ ] Docs
[ ] Collection
[ ] CLI
[X] Other

Installation method

docker development environment

Modifications

no

Ansible version

No response

Operating system

No response

Web browser

No response

Steps to reproduce

Run the updater.

Expected results

It shouldn't leave comments referring to the removed entries.

Actual results

It does.

Additional information

I printed out the produced constraint file output before and after https://github.com/ansible/awx/blob/78f345c/requirements/updater.sh#L27-L35 to verify that it's not a problem with pip-tools and found that this is a weakness of how sed is post-processing the pip-compile output.

Here's the difference of what pip-compile produces vs. how it's changed:

--- requirements/pre-sed-test.txt   2024-08-22 17:43:18.833686763 +0200
+++ requirements/post-sed-test.txt  2024-08-22 17:43:10.246719767 +0200
@@ -15,7 +15,6 @@
     # via pydantic
 ansi2html==1.9.2
     # via -r /awx_devel/requirements/requirements.in
-ansible-runner @ git+https://github.com/ansible/ansible-runner.git@devel
     # via -r /awx_devel/requirements/requirements_git.txt
 asciichartpy==1.5.25
     # via -r /awx_devel/requirements/requirements.in
@@ -78,7 +77,6 @@
     #   s3transfer
 cachetools==5.3.2
     # via google-auth
-certifi @ git+https://github.com/ansible/system-certifi.git@devel
     # via
     #   -r /awx_devel/requirements/requirements_git.txt
     #   kubernetes
@@ -144,7 +142,6 @@
     #   django-solo
     #   djangorestframework
     #   social-auth-app-django
-django-ansible-base @ git+https://github.com/ansible/django-ansible-base@devel
     # via -r /awx_devel/requirements/requirements_git.txt
 django-auth-ldap==4.6.0
     # via -r /awx_devel/requirements/requirements.in
@@ -436,7 +433,6 @@
     # via -r /awx_devel/requirements/requirements.in
 python3-openid==3.2.0
     # via social-auth-core
-python3-saml @ git+https://github.com/ansible/python3-saml.git@devel
     # via -r /awx_devel/requirements/requirements_git.txt
 pytz==2024.1
     # via

AlanCoding commented 3 weeks ago

@webknjaz this looks like it would be resolved by https://github.com/ansible/awx/pull/15317

webknjaz commented 3 weeks ago

Oh, interesting. I suppose it would. Although, I'd probably look into solving this differently. But that's sometime in the future. Maybe, modularization will yield different practices to be followed around packaging+dev env. We'll see.

ansible / awx