Page MenuHomeFreeBSD

sysutils/ansible: drop module interpreter hacks
ClosedPublic

Authored by lifanov on Mar 28 2017, 1:55 PM.

Details

Summary
Since the port creation, I was modifying all source files to replace
/usr/bin/python with whatever python_CMD was. This is ugly and unsustainable
and it was broken for a few releases now. I would like to bring the package
closer to the upstream by falling back/ defaulting to /usr/bin/python.

I would like some feedback on the plan.
Test Plan
[freebsd:vars]
ansible_python_interpreter=/usr/local/bin/python2

I tried a few modules and all seems OK.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This was done in a very strange way.

It should not define python_CMD, especially not to what it was setting it, as python_CMD is correctly defined by USES=shebangfix, and it should have been using SHEBANG_REGEX=.*\.py or something similar instead of trying to figure out where all the .py files are.

I really do not think it should install any package with a /usr/bin/python shebang.

It wasn't installing anything executable with this shebang, since distutils was already taking care of 100% of this for every user-interactive component.
What I was fixing up is the code defaulting to '/usr/bin/python' in a whole bunch of places if an interpreter ended up being undefined.

This regressed a couple of releases ago and no one complained and this brings defaults and expected behavior when managing other systems (like CentOS) more in line with official documentation and makes it less surprising to new users.

So, it was "fixing" files that are never executed ? Maybe upstream should remove the shebangs from those files.

This piece of software works by running raw commands or small generated python scripts on managed systems.
Managed systems can be anything from an F5 load balancer to a CentOS system to a FreeBSD system, etc.

In order to generate these, there is a number of settings, including ansible_python_interpreter.
The fall-through case is using /usr/bin/python, which is what 99% of the devices will have.

Example code:

if "ansible_python_interpreter" not in new_host.vars:
    py_interp = sys.executable
    if not py_interp:
        # sys.executable is not set in some cornercases.  #13585
        display.warning('Unable to determine python interpreter from sys.executable. Using /usr/bin/python default.'
                ' You can correct this by setting ansible_python_interpreter for localhost')
        py_interp = '/usr/bin/python'
    new_host.set_variable("ansible_python_interpreter", py_interp)
sysutils/ansible/files/pkg-message.in
12 ↗(On Diff #26730)

spelling: and

address matthew's comments:

fix spelling of "and"

A couple more thoughts for mat's benefit:

The shebang from module files is the base that's replaced by ansible_python_interpreter at runtime.
Adhering closer to upstream also helps users that check out local modules that are not part of the port.
After this change they will just work.

I'm convinced that this is a right and positive change and will benefit most users. I'm looking for a feedback on how to transition.

Having thought about this: the question is what should the python scripts that
ansible pushes to it's managed systems assume by default about the location of
the python interpreter?

At the moment it assumes that people are going to be managing all or mostly
FreeBSD machines, so ${LOCALBASE}/bin/python is the default, and has to be
overridden to /usr/bin/python for the non-FreeBSD ansible clients. That seems
to me a fairly reasonable assumption if you're going to be running ansible from
a FreeBSD machine in the first place.

Nikolai's arguments are cogent though: ansible clients are not necessarily
FreeBSD machines, and in a mixed environment, having usr/bin/python as the default
would make more sense. Also this should mean you can check out your ansible
playbooks and run exactly the same code from any platform.

I can't see any particularly compelling reason to prefer one of those positions
over the other. However I have two suggestions:

  • Can't you use a construct like '/usr/bin/env python'? That should work practically anywhere that can run python.
  • You could make the default interpreter setting an option in the ansible port.

In fact, I'd suggest using OPTIONS here as a way of allowing people to switch
if desired, without forcing them to. Leave the default as is, but add an option
to allow users to toggle the default easily. Implementing the current proposal
as it stands means everyone using the ansible port at the moment would have to
alter their configuration, which isn't ideal for a portrevision type update.

Revisit which is the default OPTION setting when ansible-2.3 comes out, where
the major version change would entail playbook changes anyhow?

sysutils/ansible/files/pkg-message.in
19 ↗(On Diff #26737)

That is a bad idea, it should be the content of ${PYTHON_CMD} so, LOCALBASE/bin/python2.7 by default, so that the port can get rid of the dependency on lang/python.

address mat's feedback

Yes, PYTHON_CMD is better here.

Nikolai's arguments are cogent though: ansible clients are not necessarily
FreeBSD machines, and in a mixed environment, having usr/bin/python as the default
would make more sense. Also this should mean you can check out your ansible
playbooks and run exactly the same code from any platform.

Then examples, docs, custom modules from third-party vendors, and playbooks from third-party vendors will Just Work.

I can't see any particularly compelling reason to prefer one of those positions
over the other. However I have two suggestions:

  • Can't you use a construct like '/usr/bin/env python'? That should work practically anywhere that can run python.

This negates most of the original benefit: being able to use official docs and examples and third-party modules as-is.
This can be any string, even '<<<!replace here please....1234!>>>'. This string gets used in a few places, such as both
a replacement base for ansible_python_interpreter and also the default fall-through interpreter.

  • You could make the default interpreter setting an option in the ansible port.

I don't really want to have an OPTION for this. This regressed a couple of releases ago and no one noticed. It will be even more work to keep it going if it's not the default.

Revisit which is the default OPTION setting when ansible-2.3 comes out, where
the major version change would entail playbook changes anyhow?

OK, I'll wait for a disruptive port change like this until 2.3. This way it will be easier to MFH patches to stable branches meanwhile.

This revision was automatically updated to reflect the committed changes.