search

ros-infrastructure / ros_buildfarm

ROS buildfarm based on Docker
Apache License 2.0
81 stars 96 forks source link

PEP 338 invocation of scripts #973

Closed cottsay closed 2 years ago

cottsay commented 2 years ago

This PR begins the process of replacing the current script-style invocation of ros_buildfarm code with PEP 338-style module invocation.

In particular, this PR moves all of the functionality into the ros_buildfarm Python package and introduces wrapper scripts in place of the existing scripts. There should be no change in behavior except that the scripts can now be invoked like this: python3 -m ros_buildfarm.scripts.generate_all_jobs --help.

Much of this change was done using various forms of bash violence. All of the Python wrappers that were introduced were tested with --help to ensure that the module path was correct.

Some of the existing scripts used sys.exit(main()), and these were changed to simply call main() as is the more predominant pattern.

When merging, this change should not be squashed, so that the separate commits preserve the history of the script files in their new locations.

cottsay commented 2 years ago

Rough outline of how this change was made:

  1. Move scripts to ros_buildfarm/scripts
  2. Drop exec from files now in the package
  3. Drop shbang from files now in the package
  4. Change sys.exit(main()) to main()
  5. Add appropriate __init__.py

...commit...

  1. Re-add original script files back
  2. Fix several scripts which were missing exec for some reason (???)
  3. Replace content of scripts with a run_module template
  4. Use a pile of bash for loops and seds to replace the module name within the wrapper scripts based on their filename.
  5. Try running --help on each wrapper script to ensure module names resolve.
nuclearsandwich commented 2 years ago

itshappening

When merging, this change should not be squashed, so that the separate commits preserve the history of the script files in their new locations.

Just putting this right at the bottom because half the time I forget in the heat of battle and use the default merge. If you do the same: IMO force push the intended rebase to master.