Re: [ccache] ccache problem with CCACHE_BASEDIR
On 27 July 2012 14:33, Joel Rosdahl j...@rosdahl.net wrote: [...] So, the code needs to handle x_realpath returning NULL. Eric, I've now applied your patch with some tweaks in 2df269a3121889ebcdfa5d98dfb4d675f690e039 (see http://git.samba.org/?p=ccache.git;a=commit;h=2df269a3121889ebcdfa5d98dfb4d675f690e039). Thanks! 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. [...] On the other hand, such a function would still need to expand symlinks when there are .. components in the path, which feels quite complex. -- Joel ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] ccache problem with CCACHE_BASEDIR
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.b...@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@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] ccache problem with CCACHE_BASEDIR
On 2012-06-11 07:54, Eric Blau wrote:
On 2012-06-10 16:09, Joel Rosdahl wrote:
My suggestion is that get_relative_path essentially should stay the
same and continue to assume that the paths are well-formed and
canonical. make_relative_path (the only caller of get_relative_path)
should canonicalize current_working_dir and path by calling
realpath(3). Does this sound reasonble?
Thanks for looking at this, Joel. I modified the code to do essentially
this and it is working great now. I'll send a patch to you and the list
shortly.
Hi Joel,
Sorry for the slow response, but I have a patch attached for this issue.
Please let me know if you have any comments.
Thanks,
Eric
diff -uNr ccache-3.1.7/util.c ccache-3.1.7.patched/util.c
--- ccache-3.1.7/util.c 2012-01-08 09:40:55.0 -0500
+++ ccache-3.1.7.patched/util.c 2012-07-19 12:41:37.0 -0400
@@ -993,11 +993,15 @@
int i;
const char *p;
char *result;
+ char *cpath;
if (!*to || *to != '/') {
return x_strdup(to);
}
+ cpath = x_realpath(from);
+ from = cpath;
+
result = x_strdup();
common_prefix_len = common_dir_prefix_length(from, to);
for (p = from + common_prefix_len; *p; p++) {
@@ -1021,6 +1025,7 @@
free(result);
result = x_strdup(.);
}
+ free(cpath);
return result;
}
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] ccache problem with CCACHE_BASEDIR
On 2012-06-10 16:09, Joel Rosdahl wrote: My suggestion is that get_relative_path essentially should stay the same and continue to assume that the paths are well-formed and canonical. make_relative_path (the only caller of get_relative_path) should canonicalize current_working_dir and path by calling realpath(3). Does this sound reasonble? Thanks for looking at this, Joel. I modified the code to do essentially this and it is working great now. I'll send a patch to you and the list shortly. Thanks, Eric ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
[ccache] ccache problem with CCACHE_BASEDIR
Hi folks,
I've been trying to use ccache with the CCACHE_BASEDIR option so that my fellow
team members can take advantage of compiles that have already been done by each
other from a different directory. I'm seeing ccache do inappropriate
replacements of absolute paths with relative paths. Here is an example:
I have CCACHE_BASEDIR set to /opt/eblau/comcol/Tcm6 and I'm trying to compile
from within a sub directory under that path. I have a directory with a bunch of
header files in /opt/eblau/comcol/Tcm6/prod/include. When I do a build under,
for example, /opt/eblau/comcol/Tcm6/src/core/comm/mx I see this in the ccache
log:
[2012-06-05T11:33:20.022515 21600] Working directory:
/opt/eblau/comcol/Tcm6/src/./core/comm/mx
[2012-06-05T11:33:20.022520 21600] Base directory: /opt/eblau/comcol/Tcm6
[2012-06-05T11:33:20.022525 21600] Unify mode enabled
[2012-06-05T11:33:20.022530 21600] Direct mode disabled
[2012-06-05T11:33:20.022551 21600] -g used; disabling unify mode
[2012-06-05T11:33:20.022582 21600]
dirname=/opt/eblau/comcol/Tcm6/src/./core/comm
[2012-06-05T11:33:20.022594 21600] full
path=/opt/eblau/comcol/Tcm6/src/./core/comm/mx
[2012-06-05T11:33:20.022620 21600] Source file: MxServerLink.cxx
[2012-06-05T11:33:20.022625 21600] Object file: MxServerLink.o
[2012-06-05T11:33:20.022636 21600] Running preprocessor
[2012-06-05T11:33:20.022655 21600] Executing /usr/bin/g++ -c -fno-exceptions
-Wall -Wno-deprecated -Wno-trigraphs -Wno-strict-aliasing -march=x86-64 -g
-Dlinux -D_GNU_SOURCE -D__STDC_LIMIT_MACROS -Wno-invalid-offsetof -D_REENTRANT
-fPIC -I. -I../../../../../prod/include -I/usr/src/LiS/include
-I/usr/lib/g++-include -I/usr/lib/gcc/x86_64-redhat-linux/4.1.2/include -E
MxServerLink.cxx
[2012-06-05T11:33:20.022729 21601] Unlink
/opt/cache/ccache/tmp/MxServerLi.tmp.cmdev3.21600.ii (as-tmp)
[2012-06-05T11:33:20.022815 21601] Unlink
/opt/cache/ccache/tmp/tmp.cpp_stderr.cmdev3.21600 (as-tmp)
[2012-06-05T11:33:20.058076 21600] Unlink
/opt/cache/ccache/tmp/MxServerLi.tmp.cmdev3.21600.ii (as-tmp)
[2012-06-05T11:33:20.058232 21600] Unlink
/opt/cache/ccache/tmp/tmp.cpp_stderr.cmdev3.21600 (as-tmp)
[2012-06-05T11:33:20.058257 21600] Preprocessor gave exit status 1
[2012-06-05T11:33:20.058266 21600] Failed; falling back to running the real
compiler
ccache is misinterpreting the . in the absolute path and adding on extra ..
to the -I path in the compile, causing it to fail to find the header header
files under /opt/eblau/comcol/Tcm6/prod/include.
The buggy code looks to be in get_relative_path() in util.c:
common_prefix_len = common_dir_prefix_length(from, to);
for (p = from + common_prefix_len; *p; p++) {
if (*p == '/') {
x_asprintf2(result, ../%s, result);
}
}
This code is not considering if there is a /./ in the absolute path, but is
blindly replacing this with ... Has anyone else hit this? Would a patch be
accepted for this problem if I fixed it? Any other suggestions?
Thanks,
Eric
___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache
