Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use currently installed ansible-core for devel and stable subcommands #121

Merged

Conversation

felixfontein
Copy link
Collaborator

@felixfontein felixfontein mentioned this pull request Apr 9, 2023
Copy link
Collaborator

@gotmax23 gotmax23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other small docs suggestion. Otherwise, this looks good to me. Thanks!

src/antsibull_docs/cli/antsibull_docs.py Outdated Show resolved Hide resolved
Comment on lines 296 to 300
stable_parser.add_argument('--use-installed-ansible-core', action='store_true',
help='Assumes that ansible-core is already installed and can be'
' used by calling `ansible`, `ansible-doc`, and `ansible-galaxy`'
' from $PATH.')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stable_parser.add_argument('--use-installed-ansible-core', action='store_true',
help='Assumes that ansible-core is already installed and can be'
' used by calling `ansible`, `ansible-doc`, and `ansible-galaxy`'
' from $PATH.')
help='Assumes that ansible-core is already installed and can be'
' used by calling `ansible`, `ansible-doc`, and `ansible-galaxy`'
' from $PATH.'
' By default, antsibull-docs installs ansible-core into a temporary venv.')

Copy link

@mattclay mattclay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this PR is still missing one necessary change. It doesn't look like it provides a way to preserve PYTHONPATH, which is being removed here:

del ANSIBLE_PATH_ENVIRON['PYTHONPATH']

This prevents antsibull-docs from recognizing the ansible-core that is injected into the virtual environment.

@felixfontein felixfontein merged commit 84112d1 into ansible-community:main Apr 13, 2023
@felixfontein felixfontein deleted the fake-venv-stable-devel branch April 13, 2023 04:27
@felixfontein
Copy link
Collaborator Author

@gotmax23 @mattclay thanks for reviewing this!

@mattclay
Copy link

@felixfontein Do you have an idea of when you'll be releasing a version with this change?

@felixfontein
Copy link
Collaborator Author

@mattclay first antsibull-core 2.0.0 needs to be released before antsibull-docs 2.0.0 can be released (which will contain this change). We're in the middle of preparing both; I'm right now not sure how long this will take. I hope/think/assume both will be done in a couple of weeks, but it depends a lot on when/how much @gotmax23 and me have time to work on it...

@gotmax23
Copy link
Collaborator

Is this worth backporting?

@felixfontein
Copy link
Collaborator Author

It's a feature, so no backport. stable-1 is reserved for bugfixes (so we can update antsibull-docs in ansible-core 2.15).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants