Add `--only PKG` option #76

allenh1 · 2017-10-23T19:42:37Z

This set of changes adds an option to generate a single package by name, and will also include a refactor of generate_packages.py so that it can be relocated outside the ebuild folder (this will eliminate a lot of redundancy).

ToDo:

add the --only PKG flag to generate a single package
refactor & separate gentoo-specific logic
modify regenerate_manifests to only regenerate changed packages

allenh1 · 2017-10-24T18:06:57Z

@tfoote I'm working on this right now before I look into more unit tests and making the yocto stuff better in an effort to reduce the duplicate sections of code.

My line of thought here is to make a gen_package_list function that the entry points call. This function should return the a list of the packages to be built, together with metadata. This list gets passed to an entry point specific target, and the target does with it whatever it's supposed to do (so generating ebuilds and generating manifests, or bitbake stuff).

I think this would also make things better in the future for extending things, and I can't figure out what went wrong in my head wherein I didn't write it this way the first time.

Does this sound like a good step to you?

allenh1 · 2017-10-26T02:52:39Z

Rebased on top of #77

…ntoo directory.

…handle missing dependencies.

…ages with changes.

allenh1 · 2017-10-26T21:37:46Z

@tfoote I was going to remove the Open Embedded dependency with this PR, then I noticed how many commits in I was. I'll adapt there after this one, I think.

tfoote

I've put in a few suggestions on how to cleaup/improve. Deferring the open embedded separation seems like a good decision.

tfoote · 2017-10-27T23:21:11Z

superflore/generate_installers.py

+    results = 'Generated {0} / {1}'.format(succeeded, failed + succeeded)
+    results += ' for distro {0}'.format(distro_name)
+    info("------ {0} ------".format(results))
+    print()


It's weird to use print in addition to the logging functions. Maybe just add a newline to the above?

Ya, that's definitely true. I might just remove it all together.

tfoote · 2017-10-27T23:24:49Z

superflore/generate_installers.py

+    distro_name,      # ros distro name
+    overlay,          # repo instance
+    gen_pkg_func,     # function to call for generating
+    update=True       # are we in update mode?


What does update mean here? I thought it was forcing an update but https://github.com/ros-infrastructure/superflore/pull/76/files#diff-22c1a4ad79cb71ae0b51b9ef9621bf80R60 doesn't make sense in that case.

Ah, update gets set to True to keep the ebuilds that are currently there (so quite the opposite). I'll change it back to preserve_existing, which it was previously.

tfoote · 2017-10-27T23:26:30Z

superflore/generators/ebuild/gen_packages.py



-def make_dir(dirname):
+def regenerate_pkg(overlay, pkg, distro_name=None, distro=None, update=False):


Do you really need both distro and distro_name support? That seems like it is just going to be more to test. Since it's only a one line command to get the distro. It would be better to do that from the calling instance and not have the ambiguous API.

Since it's only a one line command to get the distro. It would be better to do that from the calling instance and not have the ambiguous API.

Ya, that's true. I was going for an optimization here, but I don't think I really saved myself anything.

tfoote · 2017-10-27T23:32:23Z

superflore/generators/ebuild/run.py

+    parser.add_argument(
+        '--only',
+        help='generate only the specified package',
+        type=str


I'd recommend supporting mutiple listed packages. You can use nargs='+' https://docs.python.org/3/library/argparse.html#nargs

Note you'll slightly need to change the logic https://github.com/ros-infrastructure/superflore/pull/76/files#diff-5a7657ae13022a44815efc96706d2233R147 to iterate over the multiple arguments.

I was wondering how one would go about this. ;) Thanks for the resources. I'll definitely add this in.

…be selected.

allenh1 · 2017-10-30T14:55:37Z

@tfoote Ok, ready for round two.

@tfoote

* Creating an option to only regenerate a specified package. * Regeneration of a single package is now working. Next is refactoring. * Add unused installer list from regenerate function. * Unify printing functions * Forgot one of the print calls, and moved a variable accidently. * Added util print functions to run.py. * Make the exception handling for make_dir more strict. * Linting after rebase. * Removing duplicated logic from gen_packages function * Moved generate installers function to its own file, outside of the gentoo directory. * Remove all gentoo specific logic from gen_packages, and add logic to handle missing dependencies. * Fixed issue with unresolved dependencies not being reported. * Modify regenerate package logic to only regenerate manifests for packages with changes. * Fix regen logic for a single package. * Addresses comments by @tfoote. * Added nargs='+' to the '--only' option so that multiple packages can be selected.

allenh1 added the in progress label Oct 23, 2017

allenh1 mentioned this pull request Oct 25, 2017

Fix Linter Errors #77

Merged

allenh1 added 7 commits October 25, 2017 21:48

Creating an option to only regenerate a specified package.

e50dc76

Regeneration of a single package is now working. Next is refactoring.

043cfa2

Add unused installer list from regenerate function.

c0508a2

Unify printing functions

dba5887

Forgot one of the print calls, and moved a variable accidently.

5989711

Added util print functions to run.py.

bf9cd48

Make the exception handling for make_dir more strict.

7fadbca

allenh1 force-pushed the add-only-option branch from 26f99eb to 7fadbca Compare October 26, 2017 02:51

allenh1 force-pushed the add-only-option branch 2 times, most recently from f9ee362 to 5b9bac0 Compare October 26, 2017 03:07

Linting after rebase.

eca4561

allenh1 force-pushed the add-only-option branch from 0755800 to eca4561 Compare October 26, 2017 03:14

allenh1 added 6 commits October 26, 2017 14:14

Removing duplicated logic from gen_packages function

8a33a75

Moved generate installers function to its own file, outside of the ge…

f17eec3

…ntoo directory.

Remove all gentoo specific logic from gen_packages, and add logic to …

2be3b0b

…handle missing dependencies.

Fixed issue with unresolved dependencies not being reported.

98738b4

Modify regenerate package logic to only regenerate manifests for pack…

805f9c3

…ages with changes.

Fix regen logic for a single package.

7f5e5c9

allenh1 requested a review from tfoote October 26, 2017 21:26

allenh1 added in review and removed in progress labels Oct 26, 2017

allenh1 mentioned this pull request Oct 27, 2017

ros-lunar/mavros package missing (required by ros-lunar/mavros_extras) ros/ros-overlay#229

Open

tfoote reviewed Oct 27, 2017

View reviewed changes

allenh1 force-pushed the add-only-option branch 2 times, most recently from f1726cf to cd2ddc8 Compare October 28, 2017 17:22

Addresses comments by @tfoote.

b707825

allenh1 force-pushed the add-only-option branch from cd2ddc8 to b707825 Compare October 28, 2017 17:25

Added nargs='+' to the '--only' option so that multiple packages can …

3631b42

…be selected.

tfoote approved these changes Oct 30, 2017

View reviewed changes

allenh1 merged commit fef7c79 into master Oct 30, 2017

allenh1 deleted the add-only-option branch October 30, 2017 19:24

This was referenced Oct 31, 2017

Feature Parity for Open Embedded #79

Merged

Adding Support for Docker Library #70

Open

Refactor #27

Closed

herb-kuta-lge removed the in review label Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `--only PKG` option #76

Add `--only PKG` option #76

allenh1 commented Oct 23, 2017 •

edited

Loading

allenh1 commented Oct 24, 2017

allenh1 commented Oct 26, 2017

allenh1 commented Oct 26, 2017

tfoote left a comment

tfoote Oct 27, 2017

allenh1 Oct 28, 2017

tfoote Oct 27, 2017

allenh1 Oct 28, 2017

tfoote Oct 27, 2017

allenh1 Oct 28, 2017

tfoote Oct 27, 2017

allenh1 Oct 28, 2017

allenh1 commented Oct 30, 2017



		def make_dir(dirname):
		def regenerate_pkg(overlay, pkg, distro_name=None, distro=None, update=False):

Add --only PKG option #76

Add --only PKG option #76

Conversation

allenh1 commented Oct 23, 2017 • edited Loading

ToDo:

allenh1 commented Oct 24, 2017

allenh1 commented Oct 26, 2017

allenh1 commented Oct 26, 2017

tfoote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allenh1 commented Oct 30, 2017

Add `--only PKG` option #76

Add `--only PKG` option #76

allenh1 commented Oct 23, 2017 •

edited

Loading