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

Initial OpenEmbedded Support for ROS 2 #168

Merged
merged 80 commits into from
Jun 4, 2019

Conversation

andre-rosa
Copy link
Contributor

@andre-rosa andre-rosa commented Mar 30, 2019

Initial OpenEmbedded support for superflore

Tagging @rojkov @bulwahn @johannesschrimpf @mpthompson @dkargin @dominiquehunziker @mdhorn << Could you please take a look?

@andre-rosa
Copy link
Contributor Author

Looks like the CI failure relates to the required dependencies. Please advise on the best way to proceed.

Copy link
Collaborator

@allenh1 allenh1 left a comment

Choose a reason for hiding this comment

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

This is a lot to review. Overall, this is looking like some major beefing up of superflore.

I'd like to make a few requests, though: since this change is so big, please refrain from small changes/refactors that don't deal with superflore supporting OE. I'm more than happy to review those separately, it just makes this diff really massive and hard to parse.

In addition, please @-mention some of the meta-ros people here, since I am not the right person to review that aspect of this.

Finally, thank you all for investing your time into this project! I'm beyond excited to see superflore continuing to expand and grow.

setup.py Outdated Show resolved Hide resolved
superflore/utils.py Outdated Show resolved Hide resolved
superflore/utils.py Outdated Show resolved Hide resolved
superflore/generators/bitbake/run.py Outdated Show resolved Hide resolved
superflore/generators/bitbake/run.py Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch from d4f6fae to 2148ddc Compare April 2, 2019 01:49
@allenh1
Copy link
Collaborator

allenh1 commented Apr 2, 2019

@andre-rosa CI is running! 🎉 Unfortunately, the linter seems quite unhappy (though that's not too hard to fix usually).

Other than linting, I think your rosdep changes are needed in order to pass CI, so we'll need to include that commit hash in requirements.txt to make sure that tests are still running here & there.

Please also update the tests as needed, since you have reworked some of them (and thus some are no-longer valid).

Thanks!

@andre-rosa andre-rosa mentioned this pull request Apr 2, 2019
@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch 3 times, most recently from 1873151 to 2a2efa8 Compare April 2, 2019 23:34
@allenh1 allenh1 self-assigned this Apr 3, 2019
@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch 5 times, most recently from c477977 to c75499f Compare April 3, 2019 22:48
@allenh1
Copy link
Collaborator

allenh1 commented Apr 8, 2019

@andre-rosa looks like there's some more tests to be updated. Look in tests/test_utils.py for the dependency resolution checks -- likely just differences in names. Please update these to their proper names!

Thanks!

@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch from 52bc23c to 175d60c Compare April 8, 2019 22:19
@andre-rosa
Copy link
Contributor Author

andre-rosa commented Apr 8, 2019

@andre-rosa looks like there's some more tests to be updated. Look in tests/test_utils.py for the dependency resolution checks -- likely just differences in names. Please update these to their proper names!

Thanks!

@allenh1 << Thanks again for your review! All tests pass appropriately:

python3 -m 'nose' --exclude test_pull --exclude test_run --exclude test_logger_output
....................................................
----------------------------------------------------------------------
Ran 52 tests in 10.552s

OK

@allenh1
Copy link
Collaborator

allenh1 commented Apr 9, 2019

@andre-rosa I think that last commit should be a separate PR -- again, we have no limit on PRs, so make as many as you need to! I very much welcome them! But adding another release is not in the spirit of this PR, since it also affects Gentoo.

I'll review the rest of this change set during the day tomorrow.

Thanks again for all your work!

@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch from a25ff19 to 175d60c Compare April 9, 2019 03:45
@andre-rosa
Copy link
Contributor Author

Sounds good, thank you for reviewing, @allenh1! Please take a look at #171 .

Copy link
Collaborator

@allenh1 allenh1 left a comment

Choose a reason for hiding this comment

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

I'd still like this change set to be broken apart into a few more pieces, just so that we can keep better track of things in the git history (since I squash before merge).

I've suggested a next chunk to go for, and think that would be a good idea.

Otherwise, there's a few changes I've requested with the open embedded work.

Thanks again for your patience! I'm very excited to get these changes into superflore.

tests/test_utils.py Show resolved Hide resolved
superflore/utils.py Show resolved Hide resolved
superflore/parser.py Outdated Show resolved Hide resolved
superflore/generators/bitbake/yocto_recipe.py Outdated Show resolved Hide resolved
superflore/generators/bitbake/yocto_recipe.py Outdated Show resolved Hide resolved
superflore/generators/bitbake/yocto_recipe.py Outdated Show resolved Hide resolved
superflore/generators/ebuild/ebuild.py Outdated Show resolved Hide resolved
@andre-rosa andre-rosa force-pushed the initial_openembedded_support branch 2 times, most recently from 634f1d8 to ce4779a Compare April 11, 2019 03:25
@allenh1 allenh1 changed the title Initial openembedded support Initial Open Embedded Support for ROS 2 Apr 11, 2019
@andre-rosa
Copy link
Contributor Author

andre-rosa commented May 29, 2019

@tfoote @allenh1 << 👍 It's fully ready from our side now! I ran CI locally and it succeeded.

@allenh1
Copy link
Collaborator

allenh1 commented May 30, 2019

It's fully ready from our side now!

I'm going to need to take another look at most of it, since it's been a while, honestly. I'll try to do another full review at some point today. Note that we'll likely still need to pull chunks out.

I'm also slightly concerned about a dip in coverage, but those can likely be fixed in post (with good faith they will be done).

I ran CI locally and it succeeded.

That's good and all, but the merge button is blocked by the red CI.

@allenh1
Copy link
Collaborator

allenh1 commented May 30, 2019

@andre-rosa must have been some fluke on the build server. Ran it four times in a row so I got concerned. Seems fine now.

I'm going to do the review, then I'll have @tfoote give it a look, then we'll plan something going forward.

@allenh1
Copy link
Collaborator

allenh1 commented May 30, 2019

@andre-rosa would you please run the following for me?

 python3 -m coverage run --source=superflore -m nose && python3 -m coverage report -m

@allenh1 allenh1 self-requested a review May 30, 2019 20:07
@andre-rosa
Copy link
Contributor Author

 python3 -m coverage run --source=superflore -m nose && python3 -m coverage report -m

@allenh1

..............E..........................................
======================================================================
ERROR: Test Docker run
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/andre/projects/superflore.LGSVL/tests/test_docker.py", line 64, in test_run
    docker_instance.run()
  File "/home/andre/projects/superflore.LGSVL/superflore/docker.py", line 115, in run
    volumes=self.directory_map,
  File "/home/andre/.local/lib/python3.6/site-packages/docker/models/containers.py", line 785, in run
    detach=detach, **kwargs)
  File "/home/andre/.local/lib/python3.6/site-packages/docker/models/containers.py", line 843, in create
    resp = self.client.api.create_container(**create_kwargs)
  File "/home/andre/.local/lib/python3.6/site-packages/docker/api/container.py", line 427, in create_container
    return self.create_container_from_config(config, name)
  File "/home/andre/.local/lib/python3.6/site-packages/docker/api/container.py", line 437, in create_container_from_config
    res = self._post_json(u, data=config, params=params)
  File "/home/andre/.local/lib/python3.6/site-packages/docker/api/client.py", line 289, in _post_json
    return self._post(url, data=json.dumps(data2), **kwargs)
  File "/usr/lib/python3.6/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/lib/python3.6/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.6/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/lib/python3.6/json/encoder.py", line 180, in default
    o.__class__.__name__)
TypeError: Object of type 'Image' is not JSON serializable
-------------------- >> begin captured stdout << ---------------------
>>>> Working in temporary directory /tmp/tmp_dhj18iz
>>>> Cleaning up temporary directory /tmp/tmp_dhj18iz

--------------------- >> end captured stdout << ----------------------
-------------------- >> begin captured logging << --------------------
docker.utils.config: DEBUG: Trying paths: ['/home/andre/.docker/config.json', '/home/andre/.dockercfg']
docker.utils.config: DEBUG: Found file at path: /home/andre/.docker/config.json
docker.auth: DEBUG: Found 'auths' section
docker.auth: DEBUG: Found entry (registry='auto-gitlab.lgsvl.net:4567', username='andre.rosa')
docker.auth: DEBUG: Found entry (registry='https://index.docker.io/v1/', username='andregrosa')
docker.api.build: DEBUG: Looking for auth config
docker.api.build: DEBUG: Sending auth config ('auto-gitlab.lgsvl.net:4567', 'https://index.docker.io/v1/')
urllib3.connectionpool: DEBUG: http://localhost:None "POST /v1.35/build?q=False&nocache=False&rm=False&forcerm=False&pull=False HTTP/1.1" 200 None
urllib3.connectionpool: DEBUG: http://localhost:None "GET /v1.35/images/0dcb04dcb0d0/json HTTP/1.1" 200 1572
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 57 tests in 105.635s

FAILED (errors=1)

@allenh1
Copy link
Collaborator

allenh1 commented May 30, 2019

@andre-rosa you can skip that test if you want.

@andre-rosa
Copy link
Contributor Author

@andre-rosa you can skip that test if you want.

@allenh1

.................................................
----------------------------------------------------------------------
Ran 49 tests in 35.221s

OK
Name                                               Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------
superflore/CacheManager.py                            20      0   100%
superflore/PackageMetadata.py                         25      0   100%
superflore/TempfileManager.py                         34      6    82%   48-56
superflore/__init__.py                                10      4    60%   5-8
superflore/docker.py                                  78     57    27%   28-31, 34-36, 39, 42, 45-51, 56-69, 72, 75, 78-88, 91-129, 134
superflore/exceptions.py                              15      2    87%   28, 33
superflore/generate_installers.py                     61      2    97%   45-46
superflore/generators/__init__.py                      0      0   100%
superflore/generators/bitbake/__init__.py              3      1    67%   3
superflore/generators/bitbake/gen_packages.py        107     86    20%   39-114, 121-176, 183-191, 197
superflore/generators/bitbake/oe_query.py            104     91    12%   24-44, 47-49, 53-58, 61-96, 99-121, 124, 127-136, 140-150, 154
superflore/generators/bitbake/ros_meta.py             45     34    24%   26-30, 33-39, 42-66, 69-70, 73, 76-78
superflore/generators/bitbake/run.py                 138    114    17%   42-229
superflore/generators/bitbake/yocto_recipe.py        531    462    13%   67-124, 127-131, 134-146, 149-154, 157-159, 162-168, 171-177, 180-186, 189-195, 198-204, 207-213, 221-224, 229-231, 234-236, 239, 242-244, 248, 252, 260-276, 281-288, 293-301, 305-362, 369-499, 503-511, 515-516, 522-645, 649-692, 696-712, 716-729, 733-782, 786-794
superflore/generators/ebuild/__init__.py               3      1    67%   3
superflore/generators/ebuild/ebuild.py               185     19    90%   96-99, 126-131, 146, 176-180, 186, 193, 206-210
superflore/generators/ebuild/gen_packages.py         141    113    20%   47-120, 126-138, 144-189, 194-211, 214, 217
superflore/generators/ebuild/metadata_xml.py          33      0   100%
superflore/generators/ebuild/overlay_instance.py      43     31    28%   29-34, 37-45, 50-73, 76-77
superflore/generators/ebuild/run.py                  119     97    18%   40-188
superflore/parser.py                                  15      0   100%
superflore/repo_instance.py                           70     51    27%   31-52, 59-68, 71-79, 85-86, 92, 98-99, 105, 108-123, 126
superflore/rosdep_support.py                          37      3    92%   44, 88-89
superflore/test_integration/__init__.py                0      0   100%
superflore/test_integration/gentoo/__init__.py         3      1    67%   3
superflore/test_integration/gentoo/build_base.py      28     19    32%   26-28, 33, 38-54
superflore/test_integration/gentoo/main.py            32     26    19%   24-80
superflore/utils.py                                  185     27    85%   56-59, 63-77, 81-92, 107, 217-218, 222, 271, 276
--------------------------------------------------------------------------------
TOTAL                                               2065   1247    40%

@allenh1
Copy link
Collaborator

allenh1 commented May 30, 2019

@andre-rosa I'd really like to see a bit more coverage for oe_query.py.

superflore/generators/bitbake/oe_query.py            104     91    12%   24-44, 47-49, 53-58, 61-96, 99-121, 124, 127-136, 140-150, 154

This PR reduces the total coverage of the repository by around 10%, which makes sense (since it's adding so many things). Do you think you could add some tests for that file?


@tfoote do you have any opinions on this?

Copy link
Collaborator

@allenh1 allenh1 left a comment

Choose a reason for hiding this comment

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

Just a few things that stick out to me at the moment. I'm in favor of not dividing this more.

superflore/generators/bitbake/ros_meta.py Show resolved Hide resolved
superflore/generators/bitbake/ros_meta.py Show resolved Hide resolved
superflore/generators/bitbake/run.py Show resolved Hide resolved
superflore/generators/bitbake/yocto_recipe.py Show resolved Hide resolved
@andre-rosa
Copy link
Contributor Author

@andre-rosa I'd really like to see a bit more coverage for oe_query.py.

superflore/generators/bitbake/oe_query.py            104     91    12%   24-44, 47-49, 53-58, 61-96, 99-121, 124, 127-136, 140-150, 154

This PR reduces the total coverage of the repository by around 10%, which makes sense (since it's adding so many things). Do you think you could add some tests for that file?

Definitely; will do it.

Name                                               Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------
superflore/generators/bitbake/oe_query.py            104     12    88%   58, 70, 121, 140-150, 154
...
--------------------------------------------------------------------------------
TOTAL                                               2065   1168    43%
@andre-rosa
Copy link
Contributor Author

@allenh1 << Thanks a bunch for reviewing! Please take another look.

...................................................
----------------------------------------------------------------------
Ran 51 tests in 36.478s

OK
Name                                               Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------
superflore/CacheManager.py                            20      0   100%
superflore/PackageMetadata.py                         25      0   100%
superflore/TempfileManager.py                         34      6    82%   48-56
superflore/__init__.py                                10      4    60%   5-8
superflore/docker.py                                  78     57    27%   28-31, 34-36, 39, 42, 45-51, 56-69, 72, 75, 78-88, 91-129, 134
superflore/exceptions.py                              15      2    87%   28, 33
superflore/generate_installers.py                     61      2    97%   45-46
superflore/generators/__init__.py                      0      0   100%
superflore/generators/bitbake/__init__.py              3      1    67%   3
superflore/generators/bitbake/gen_packages.py        107     86    20%   39-114, 121-176, 183-191, 197
superflore/generators/bitbake/oe_query.py            104     12    88%   58, 70, 121, 140-150, 154
superflore/generators/bitbake/ros_meta.py             45     34    24%   26-30, 33-39, 42-66, 69-70, 73, 76-78
superflore/generators/bitbake/run.py                 138    114    17%   42-229
superflore/generators/bitbake/yocto_recipe.py        531    462    13%   67-124, 127-131, 134-146, 149-154, 157-159, 162-168, 171-177, 180-186, 189-195, 198-204, 207-213, 221-224, 229-231, 234-236, 239, 242-244, 248, 252, 260-276, 281-288, 293-301, 305-362, 369-499, 503-511, 515-516, 522-645, 649-692, 696-712, 716-729, 733-782, 786-794
superflore/generators/ebuild/__init__.py               3      1    67%   3
superflore/generators/ebuild/ebuild.py               185     19    90%   96-99, 126-131, 146, 176-180, 186, 193, 206-210
superflore/generators/ebuild/gen_packages.py         141    113    20%   47-120, 126-138, 144-189, 194-211, 214, 217
superflore/generators/ebuild/metadata_xml.py          33      0   100%
superflore/generators/ebuild/overlay_instance.py      43     31    28%   29-34, 37-45, 50-73, 76-77
superflore/generators/ebuild/run.py                  119     97    18%   40-188
superflore/parser.py                                  15      0   100%
superflore/repo_instance.py                           70     51    27%   31-52, 59-68, 71-79, 85-86, 92, 98-99, 105, 108-123, 126
superflore/rosdep_support.py                          37      3    92%   44, 88-89
superflore/test_integration/__init__.py                0      0   100%
superflore/test_integration/gentoo/__init__.py         3      1    67%   3
superflore/test_integration/gentoo/build_base.py      28     19    32%   26-28, 33, 38-54
superflore/test_integration/gentoo/main.py            32     26    19%   24-80
superflore/utils.py                                  185     27    85%   56-59, 63-77, 81-92, 107, 217-218, 222, 271, 276
--------------------------------------------------------------------------------
TOTAL                                               2065   1168    43%

allenh1
allenh1 previously approved these changes May 31, 2019
Copy link
Collaborator

@allenh1 allenh1 left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks for all the patience with this patch!

@allenh1 allenh1 requested a review from tfoote May 31, 2019 19:21
@allenh1
Copy link
Collaborator

allenh1 commented May 31, 2019

@tfoote Please give this a look.

@andre-rosa
Copy link
Contributor Author

Fixes #167

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks for pushing on this. This will be a great new capability to have.

@allenh1 allenh1 merged commit 7766e27 into ros-infrastructure:master Jun 4, 2019
@andre-rosa andre-rosa deleted the initial_openembedded_support branch June 4, 2019 16:23
@andre-rosa
Copy link
Contributor Author

Thank you @allenh1 and @tfoote for your reviews! 👍

@andre-rosa andre-rosa mentioned this pull request Jun 18, 2019
4 tasks
zffgithub pushed a commit to zffgithub/superflore that referenced this pull request Apr 11, 2023
* Unify dependency resolution between ebuild and bitbake

* Point overlay meta repository to ros/meta-ros

* Fix a exception when --skip-keys is not provided

* Add build{tool}_export_depends to DEPENDS

* Update DEPENDS format to OE recipe generation scheme rev 9

* Remove dependency groups handling

* Add ROS_BPN to the generated recipes

* Set DISTRO in generated-ros-distro.inc

* Remove packagegroup-ros-world.bb as per OE recipe generation scheme rev 25

* Set ROS_SUPERFLORE_GENERATION_DATETIME to last-modified timestamp of distro cache

* Bump rosdep dependency to >= 0.15.2

* Update comments as per OE recipe generation scheme rev 30

* Translate licenses
* Add ROS_SUPERFLORE_GENERATED_RECIPES_FOR_COMPONENTS to rosdistro.conf

* Filter out random branch name from the change summary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants