fix: Fixed has target check taking ages for large projects

[jmachowinski]

The old implementation took up to 30 seconds for packages, were a lot of targets existed. This is a common case for interface packages containing a lot of messages.

[wjwwood]

After reading https://stackoverflow.com/questions/23849953/how-to-check-existence-of-the-target-in-a-makefile, I'm wondering if we shouldn't also use --dry-run and maybe use the strategy suggested there where we check the output of the data base with --print-data-base? I'm not certain it provides more benefits, but maybe it would help to avoid unintended (and in some cases expensive time-wise) side effects.

With that suggestion aside, there's just a few small technical questions and requests before this is lgtm.

[wjwwood]

I tested this on macos and I didn't notice or see any issues building the ros2.repos up to rclcpp and a little past that.

[jmachowinski]

I had some issues with this patch, it does not seem to work in special situations.
I currently don't have time to look into it, therefore I will convert this PR into a draft.

[cottsay]

What sort of behavior does this exhibit if the CMake generator is Visual Studio? Also, what if the system's language is not set to English - is make's output localized?

[jmachowinski]

What sort of behavior does this exhibit if the CMake generator is Visual Studio?

Unchanged

Also, what if the system's language is not set to English - is make's output localized?

AFAIK make is not localized, at least it is not on my german localized ubuntu

[jmachowinski]

After reading https://stackoverflow.com/questions/23849953/how-to-check-existence-of-the-target-in-a-makefile, I'm wondering if we shouldn't also use --dry-run and maybe use the strategy suggested there where we check the output of the data base with --print-data-base? I'm not certain it provides more benefits, but maybe it would help to avoid unintended (and in some cases expensive time-wise) side effects.

--dry-run generated a lot of not needed output, which is not done by the question mode. As I don't check on the return code, but parse the std_err instead, we should be fine

[jmachowinski]

This version is working, and tested. Additional tests would be appreciated though, as I only have a linux machine.

[cottsay]

Beyond the linting errors that CI identified, I am also concerned that this change is coupled to GNU Make. The -q flag has a different (and incompatible) meaning on BSD: https://man.freebsd.org/cgi/man.cgi?make(1)

That said, BSD isn't an explicitly supported platform, but it still doesn't feel good to break it just for an optimization. I'll do a little more poking around to see if I can see another solution.

[jmachowinski]

@cottsay ping

[cottsay]

It looks like the -n flag is available on all of the platforms I tested. It prints the command that WOULD be executed for a target, but importantly returns non-zero if the target doesn't exist.

It appears to be a lot faster than help in my tests. Would that work for you?

[jmachowinski]

I just testet -n vs -q , on our system -n seems to be slower but its still a major improvement over the current state :
With -q

Starting >>> cm_interfaces
Finished <<< cm_interfaces [2.87s]                     

Summary: 1 package finished [3.59s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [2.93s]                     

Summary: 1 package finished [3.67s]

with -q

Starting >>> cm_interfaces
Finished <<< cm_interfaces [4.61s]                     

Summary: 1 package finished [5.34s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [5.41s]                     

Summary: 1 package finished [6.14s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [4.56s]                     

Summary: 1 package finished [5.29s]

I still wonder, about the impact of the text output in colcon though...

time make -q install

real    0m0,343s
user    0m0,330s
sys     0m0,011s

time make -n install
real    0m0,960s
user    0m0,812s
sys     0m0,142s

The time spend in make increases only about 600ms, vs 1.6 seconds in colcon.