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

Remove wayward "shbangs" in Python 3 library files #2917

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

portante
Copy link
Member

And we also remove the unnecessary cli/getconf.py file.

This work was identified while reviewing PR #2901.

@portante portante added bug Agent Server Code Infrastructure packaging Issues related to software packaging labels Jun 25, 2022
@portante portante added this to the v0.72 milestone Jun 25, 2022
@portante portante requested review from ndokos and webbnh June 25, 2022 01:42
@portante portante self-assigned this Jun 25, 2022
And we also remove the unnecessary `cli/getconf.py` file.

This work was identified while reviewing PR distributed-system-analysis#2901.
@portante portante added the Backport PRs with this label should be considered for back porting to previous release branches label Jun 25, 2022
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

I guess I have no problem with this, but a quick search suggests that we have 69 such files...why do we address only five of them here?

And, why is removing the line (as opposed to setting it to something "standard") the proper fix?

@dbutenhof
Copy link
Member

I guess I have no problem with this, but a quick search suggests that we have 69 such files...why do we address only five of them here?

And, why is removing the line (as opposed to setting it to something "standard") the proper fix?

These are all I find searching from lib/pbench; what are you finding?

These are library modules never executed directly by the shell, and the bash wrappers (either direct click wrappers or static wrappers ala agent/util-scripts/pbench-tool-meister-start) are properly shebanged.

@webbnh
Copy link
Member

webbnh commented Jun 27, 2022

These are library modules never executed directly by the shell

Ah! Perhaps that's the answer.

These are all I find searching from lib/pbench; what are you finding?

Aside from a few in server/bin there are a bunch in each of agent/*-scripts and a few other odds and ends.

@webbnh
Copy link
Member

webbnh commented Jun 27, 2022

And, FWIW, here are the shbangs that I found:

   1 #!/bin/env python
   5 #!/usr/bin/python
   1 #! /usr/bin/env python
  55 #!/usr/bin/env python

So, we're about 90% consistent....

@ndokos
Copy link
Member

ndokos commented Jun 28, 2022

The ones under /opt/pbench-agent/{bin,tool-scripts,util-scripts} are properly "mangled" when the RPM is created: the mangling tool seems to handle bin-type directories, but it leaves /opt/pbench-agent/lib alone: it doesn't touch those. That is not to say that we should not be consistent of course.

@webbnh
Copy link
Member

webbnh commented Jun 28, 2022

The ones under /opt/pbench-agent/{bin,tool-scripts,util-scripts} are properly "mangled" when the RPM is created: the mangling tool seems to handle bin-type directories, but it leaves /opt/pbench-agent/lib alone: it doesn't touch those.

I suspect that it is keying off the eXecute bit on the file and that the directory has nothing to do with it.

If that's the case, then we missed one -- agent/stockpile/roles/openstack_common/files/openstack-config-parser.py -- but perhaps that will be addressed by a separate effort....

@webbnh webbnh merged commit 52e8313 into distributed-system-analysis:main Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Backport PRs with this label should be considered for back porting to previous release branches bug Code Infrastructure packaging Issues related to software packaging Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants