micro_ros_setup icon indicating copy to clipboard operation
micro_ros_setup copied to clipboard

Published 20 hours ago • verified publisher icon micro-ROS

Switch to ar M-script to build libmicroros.a

Open gavanderhoorn opened this issue 3 years ago • 4 comments

As per subject.

Bit of a wip as I can't test this myself right now, and have copied the changes from my development machine verbatim.

Last two commits are technically off-topic, I'd be OK with dropping them. Diff without them: here.

I'm uncertain about whether ranlib would still be needed, so 905f5081ba6a7386f67e2d20d5a9cd7cc53d4a59 removes it. If needed, that could easily be reverted.

This is for #564.

gavanderhoorn avatar Aug 30 '22 11:08 gavanderhoorn

As I wrote in #564, I'm not aware of any issues with linking (using ar like this instead of what was there before), but obviously don't have the breadth of different platforms to test this on.

gavanderhoorn avatar Aug 30 '22 11:08 gavanderhoorn

This looks very interesting, thanks @gavanderhoorn.

We will take a look when we have some time. All the micro-ROS modules use this approach so maybe all of them shall be modified if this approach is faster or better in any sense.

pablogs9 avatar Sep 05 '22 05:09 pablogs9

It's certainly faster for me.

I've described some of the possible drawbacks in #564. There could be / likely are more I'm not aware of.

gavanderhoorn avatar Sep 05 '22 14:09 gavanderhoorn

Anecdotal -- and no replacement for proper testing -- but I've been using (a variant of) this approach for the past couple months and I've not ran into any issues.

Could be due to platform specifics, but I thought I'd provide the data point in any case.

gavanderhoorn avatar Oct 25 '22 13:10 gavanderhoorn

Hi @pablogs9, I'm trying to close (or get merged) a nr of my (still) open PRs and this one is on the list.

Would you guys be interested in merging it? Otherwise I'll close it.

gavanderhoorn avatar Feb 15 '23 08:02 gavanderhoorn

Please do not close, we will address it ASAP

pablogs9 avatar Feb 15 '23 08:02 pablogs9

With this approach, we will add all the separated objects to the M-script. This will make it slower but its needed.

I guess (as I haven't tested it) this would almost completely negate the advantage of having ar manage everything.

Feel free to close if that's the case.

gavanderhoorn avatar Feb 16 '23 10:02 gavanderhoorn

I guess (as I haven't tested it) this would almost completely negate the advantage of having ar manage everything.

Yep, just tested it locally and the processing time is not worth it with this approach. Guess we are closing this for now.

Acuadros95 avatar Feb 16 '23 11:02 Acuadros95