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

Fallback on PYTHON path in Makefile #13896

Merged
merged 1 commit into from
Apr 24, 2023
Merged

Conversation

jjwatt
Copy link
Contributor

@jjwatt jjwatt commented Apr 20, 2023

SUMMARY

If I understand correctly, this change should make '$(PYTHON)' work how we want it to everywhere. Before this change, on develpers' machines that don't have a 'python3.9' in their path, make would fail. With this change, we will prefer python3.9 if it's available, but we'll take python3 or python otherwise.

I think setting this might be an artifact from having several minor versions of Python 3 available. It may not even still be relevant. Let me know if I'm misunderstanding this or if there's somewhere where this will fail. My main goal is to avoid make failing because a developer does not have a bin named python3.9 in their path (or the equiv symlink or w/e). Like, maybe I have python3 and python3.11 and then I have to set PYTHON myself for running make. I think we can find something that works for these cases.

I really think this does what we mean for all cases: make from dev machine, make from container, make from scripts. It will even run the venv path one if started from venv.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
  • UI
  • Collection
  • CLI
  • Docs
  • Other
AWX VERSION

ADDITIONAL INFORMATION

Copy link
Member

@shanemcd shanemcd left a comment

Choose a reason for hiding this comment

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

🍰

@jjwatt jjwatt force-pushed the jjwatt-pyver branch 4 times, most recently from 71c1fb5 to f56e802 Compare April 21, 2023 12:03
@jjwatt
Copy link
Contributor Author

jjwatt commented Apr 21, 2023

Hah. This is a fun one. So, whatever shell (version of sh) that GitHub runs did not like my trick with command -v python3 python3.9 It works it a lot of shells because it will return the first full path that actually exists. However, apparently the shell that runs from GH Actions -> ci.yml -> make -> ansible -> build.yml -> make print-VERSION is one that does not work that way! Instead it returns blank/empty if the first command can't be found :). So, that was causing the Permission Denied error I was getting in the awx-operator CI.

I fixed it by using a for loop in the shell macro instead of relying on my assumed command -v behavior. I still use command -v but in a loop, and it won't error out on missing commands.

I will clean this up and squash.

- Change default PYTHON in Makefile to be ranked choice
- Fix `PYTHON_VERSION` target that expects just a word
- Use native GNU Make `$(subst ,,)` instead of `sed`
- Add 'version-for-buildyml' target to simplify ci

If I understand correctly, this change should make
'$(PYTHON)' work how we want it to everywhere. Before
this change, on develpers' machines that don't have
a 'python3.9' in their path, make would fail. With this
change, we will prefer python3.9 if it's available, but
we'll take python3 otherwise.
@TheRealHaoLiu
Copy link
Member

related to #13737

@jjwatt jjwatt merged commit 2ce9440 into ansible:devel Apr 24, 2023
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