+1 the logic looks fine. My first interpretation of base_path from the name was incorrect, but I can't come up with a better suggestion. It's more closely related to something like the install prefix.

please don't merge yet

thanks for waiting. The logic looks good to me too.

While I was investigating something I added a couple of tests in #18 which check that the expected path is being returned.

On the name, "prefix path" might be clearer than "base path". That's the term used in the environment variable AMENT_PREFIX_PATH and in other documentation.

As for the Python API, maybe add a return_prefix_path=False argument to get_resource that would make it return a tuple when requested?

Renamed to prefix_path.

Regarding the Python API I am not sure that a different return value structure based on the arguments is a good thing. I would expect that this would break user expectation quite often. But we can make the decision independently of this PR - maybe talk about it in an upcoming meeting.

I agree that changing the return type based on the argument would lead to unexpected breakages when refactoring. Thinking about this I think that there's 3 potential queries that one might want. The content, the full path, or the prefix_path. This approach covers 2 of the 3, but actually requires reading in the full content, which might be large, just to return the prefix_path. I think that 3 separate calls might make sense for both languages. Unless we're worried about race conditions there's no need to bundle the queries.

Regarding the full path, I get the impression that it's not something the user should be getting access to since it's kind of an implementation detail of the index. Returning the prefix path of a found resource is useful for knowing which prefix the resource came from in the case that there are many prefixes, but I don't think the user is expected to manipulate the resources themselves. I didn't see it explicitly written in the documentation but I think I have gotten this impression from conversations in the past

About having separate vs bundled functions, the python API has has_resource which separately returns the prefix path but not the content (the cmake version is just boolean return). I would still see value in returning the prefix path in addition to the content in get_resource, just for not wasting time finding the same resource twice if you want both the content and the prefix, but there might be other ways around that.

I suppose that depending on how strongly we feel about keeping the functions separate, it may end up affecting this PR. but I agree that once that decision is made about how to approach exposing the functionality (maybe it doesn't have to be the same across the languages), the python implementation details can be discussed independently

@tfoote I don't think that splitting these queries into three separate functions is a good idea. The information returned is based on the environment and filesystem. There is no way to ensure atomic operation across those functions therefore the individual results of them might be inconsistent. Therefore I don't think it is a good API design to separate them.

I added C++ has_resource despite the fact that we don't have any use case to check for a resource without getting the content yet.

I also updated Python get_resource to always return a tuple instead of just the content.

I reverted the change to the copyright year since the code was written in 2015 (coming from the other file). So it shouldn't be 2016.

Should I wait for explicit approvals for the referenced PRs or are they implicitly ok to be merged too?

can this branch be deleted?

Absolutely, done.