Simplify FindResource logic, if possible

[EricCousineau-TRI]

Right now, the logic in FindResource seems relatively complex. See #7774 for more information.
Given that we now have the ability to get the root directory from the shared library, it seems like the logic could be stripped down.

It'd be nice if the following could be done:

• Don't make this feature overly complicated. Any complex path searching should be done by something else, like ROS packages.
• Don't use a list of candidate directories. Only use one candidate directory, the "one true path".
• If a user makes a suggestion for a path, we should try and make this transparent (ideally not appending / reformatting it) so that expectations match usage.
• Any behind-the-scenes activity should be somewhat be debuggable without having to modify the C++ code (e.g. tell me all of the paths you looked at when trying to find a file, or guarantee that your searching only concatenates candidates with the exact string I supplied, no extra stuff).

Granted, these suggestions may skip over some details, such as the motivating points for some of the things (like persistent candidate directories, etc.).

[jwnimmer-tri]

Both in #7774 and this issue, I don't understand what the difficulty / problem / complaint is getting at. Is the API contract unclear? Are the semantics unhelpful for certain use cases? Is the implementation difficult to follow? Maybe you can swing by and try to explain in person.

[fbudin69500]

I agree with Eric that the API could be improved. It is not that it is unclear from reading the documentation, but that it is more complex than it should be, and that makes it more difficult to maintain and debug.

• It would be good to get the list of all paths that are searched when calling GetResourceSearchPaths(), including the one given by the environment variable and the one given by the path to the sentinel file if it is found. This would indeed help debugging.
• Editing the path of the resource file inside the FindResource() function to remove drake (for backward compatibility reasons) complicates the code (what @EricCousineau-TRI called 'behind the scenes activity').
• Having "one true path" would make sense, but I don't think it is very important since I haven't seen any use-case yet where this causes a problem. Setting a "true path" on initialization is appealing, but initialization of a library can be tricky: It can easily create bugs that are hard to track.
• One important improvement would be to, in my opinion, either to force this to be used only when this is in a shared object (It is confusing sometimes when debugging to realize that the reason why something is not found is because the tested executable was not linked to libdrake.so and therefore cannot find the resources based on its path), or a simpler improvement would be to add a function in this module to return false if libdrake.so was found or the path to the library which would be empty if it is not found. Either of these solutions would help with debugging.

[EricCousineau-TRI]

Per f2f with @jwnimmer-tri, some additional thoughts:

• For debugging, it may help if the message includes all of the candidate paths that it searched. Then we probably wouldn't need a public GetResourceSearchPaths() either. (I believe I had seen this info from catkin_find, which was super useful when debugging errors.)
• We may want to consider deprecating AddResourceSearchPath is users do not need this (assuming that this function was added for Python, where DRAKE_RESOURCE_ROOT would not suffice)
• We may want to consider deprecating DRAKE_RESOURCE_ROOT (depending on how downstream users use this)

The deprecation portions are lower priority; adding more info the error message seems like some low-hanging fruit to address the root problem (debugging when things go wrong).

@fbudin69500, can I ask for your thoughts on these?
They seem to be generally in-line with what you mentioned, but could allow us to keep the appending of drake/ for internally generated (or even externally generated) candidates to remain, since all the paths it had considered are visible (and thus easier to see where things may have gone wrong).

Setting a "true path" on initialization is appealing, but initialization of a library can be tricky: It can easily create bugs that are hard to track.

This may not be relevant to future solutions, but just to check, by "initialization" do you mean global-scope initialization, or just static-storage initialization (e.g. the first time FindResource is called)? My intent was to imply the latter - the only difference between the present and suggested functionality is if the user changes directory after having called FindResource.

One important improvement would be to, in my opinion, either to force this to be used only when this is in a shared object...

Can I ask what "this" is? FindResource, or the internal candidate directory generation, using libdrake.so?
That being said, I think if we have the error message print out candidate paths, it may be easier to posit this -- it could even indicate that it's not using libdrake.so, since this is something only a developer would really see.

[hongkai-dai]

@EricCousineau-TRI could you update the status on this issue? Thanks!

[EricCousineau-TRI]

Updated - thanks!

[EricCousineau-TRI]

In addressing / creating #11111, some add'l complaints:
Some paths have to have drake/ in them. Others do not. This should be normalized, for both internal and external API.

We should also just ditch an user mutation of search paths. They should always work, period, and should never change. Any user delegation should happen as part of ROS package path-like URIs (#10531).

[jwnimmer-tri]

@EricCousineau-TRI Since the above was written up, we've submitted various cleanups like #11261. Is there anything still left to do here, or can we close it? (Or at least, make it more specific.)

I don't think we can cull any more features. Maybe you still want some more error message additions, though those are also much improved already.

We should also just ditch an user mutation of search paths. They should always work, period, and should never change.

Done in #11841.

Some paths have to have drake/ in them. Others do not. This should be normalized, for both internal and external API.

Now that we have find_runfiles.h vs find_resource.h, is this still an active complaint? If so, please clarify what you mean -- I don't understand.

[EricCousineau-TRI]

Yup, everything is addressed. Still wish that DRAKE_RESOURCE_ROOT referred to /path/to/install/drake rather than /path/to/install, but as you pointed out, find_runfiles should cover wanting to find other things, and the fact that we specify /drake can be seen as legacy.

Closing - thanks for the ping!