[ccache] ccache problem with CCACHE_BASEDIR

[Joel Rosdahl]

Hi,

It seems you forgot to make cwd_canonicalized static? Currently, it
has no effect.

Also, the test suite crashes with your patch. I recommend running the
test suite during development even for minor patches (and for bonus
points: add new tests for the new functionality :-)). The crash
happens because realpath returns NULL for nonexisting paths, and the
test suite sets up fake paths for some parameters; this could happen
in real usage as well. So, the code needs to handle x_realpath
returning NULL.

Thinking more about this, it would probably be better to (instead of
realpath) use some function that doesn't expand symlinks and doesn't
care whether the path exists or not. I'm not aware of such an existing
(and portable) function, though, so we likely will have to write it
ourselves.

-- Joel

On 20 July 2012 19:45, Eric Blau <eric.blau at tekelec.com> wrote:
> On 2012-07-20 13:09, Joel Rosdahl wrote:
>>
>> Thanks. Two things:
>>
>> 1. The patch canonicalizes the "from" parameter, but I think that the
>> "to" parameter should be canonicalized as well, for two reasons: a)
>> realpath() expands symlinks, and comparing (see
>> common_dir_prefix_length) an expanded path with an unexpanded path
>> won't work well when symlinks are present. b) The "to" parameter
>> (which for instance may be a path given on the command line) could
>> also contain path elements that could benefit from canonicalization
>> (to increase cache hits).
>>
>> 2. The patch canonicalizes the "from" parameter every time
>> get_relative_path is called, which is unnecessary since it's always
>> the same. Therefore, as mentioned earlier, I think that its' better to
>> do the canonicalization in make_relative_path where
>> current_working_dir can be canonicalized when created.
>
>
> Thanks for the comments. Please discard the previous patch. This attached
> version of the patch should be better. It canonicalizes the current working
> directory once and canonicalizes the path each time get_relative_path is
> called.
>
> Thanks,
> Eric
>
>
> _______________________________________________
> ccache mailing list
> ccache at lists.samba.org
> https://lists.samba.org/mailman/listinfo/ccache
>