Bug 11171 - Enhance --ccache-skip (don't hash the option)
Summary: Enhance --ccache-skip (don't hash the option)
Alias: None
Product: ccache
Classification: Unclassified
Component: ccache (show other bugs)
Version: 3.2.1
Hardware: All All
: P5 enhancement
Target Milestone: ---
Assignee: Joel Rosdahl
QA Contact: Joel Rosdahl
Depends on:
Reported: 2015-03-18 14:48 UTC by Mark Starovoytov
Modified: 2016-06-09 19:42 UTC (History)
0 users

See Also:

proposed patch (3.53 KB, patch)
2015-03-18 14:48 UTC, Mark Starovoytov
no flags Details
proposed patch (4.30 KB, patch)
2015-03-18 19:38 UTC, Mark Starovoytov
no flags Details
proposed patch (15.07 KB, patch)
2015-03-20 12:56 UTC, Mark Starovoytov
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Starovoytov 2015-03-18 14:48:54 UTC
Created attachment 10879 [details]
proposed patch

Enhanced --ccache-skip functionality to actually exclude an option that follows it from hashing.

Attached is the patch to implement the functionality.
Comment 1 Mark Starovoytov 2015-03-18 19:38:11 UTC
Created attachment 10883 [details]
proposed patch

Uploading new version of patch:
- there was an issue with running preprocessor step from within calculate_object_hash(). The fix is not as beautiful as I would like it to be, but I'm not sure there's a better way...
Comment 2 Joel Rosdahl 2015-03-19 21:01:41 UTC
Thanks for the patch.

I haven't looked closely at it yet, but it makes the test suite uncompilable:

> % make test
> [..]
> gcc -DHAVE_CONFIG_H  -DSYSCONFDIR=/usr/local/etc -I. -I.  -MD -MP -MF .deps/test_test_argument_processing.c.d -g -O2 -Wall -W -Werror -c -o test/test_argument_processing.o test/test_argument_processing.c
> test/test_argument_processing.c: In function ‘suite_argument_processing’:
> test/test_argument_processing.c:38:13: error: too few arguments to function ‘cc_process_args’
>   CHECK(!cc_process_args(orig, &preprocessed, &compiler));
>              ^
> In file included from test/test_argument_processing.c:23:0:
> ./ccache.h:229:7: note: declared here
>  bool cc_process_args(struct args *args, struct args **preprocessor_args,
>        ^
Comment 3 Mark Starovoytov 2015-03-19 21:22:10 UTC
My bad. Will update the test suite tomorrow.
Do you want me to update the docs as well?
Comment 4 Mark Starovoytov 2015-03-20 12:56:01 UTC
Created attachment 10895 [details]
proposed patch

Fixed test suite compilation.
Added test suite test for "--ccache-skip" option
Comment 5 Joel Rosdahl 2015-05-01 10:32:37 UTC
Thanks for the patch! Some comments:

1. The current implementation approach reorders the compiler arguments, which doesn't feel acceptable. For example,

    cc -c --ccache-skip -Ifoo -Ibar file.c

will be rewritten to 

    cc -c -Ibar -Ifoo file.c

which changes semantics of the compilation since "-I" options are used in left-to-right order by the compiler. The argument order needs to be preserved.

cc_process_args has become quite complex during the years and adding yet another argument list (nonhash_args) makes it even more complex (although just slightly). I think that some refactoring is needed to introduce "non-hashed arguments" in a good way. One idea is to add one or several type field to "struct args" and let cc_process_args set the new fields for different types of arguments. One field (or bits in a bitmask) could say if it's a preprocessor argument, compiler argument or both. Another could say whether the argument should be hashed or not. Etc.

However, if you can come up with a good way of solving the "nonhashed arguments" functionality without refactoring, that would probably also be OK.

2. Thinking a bit more about this, I think that I would prefer another option than --ccache-skip for excluding an argument from the hash. The reason is that one might want to skip hashing an argument but still want to let ccache interpret the argument specially. Or the other way around. Perhaps the new option could be named --ccache-dont-hash? And maybe --ccache-skip-and-dont-hash could have both effects?

3. Documentation (MANUAL.txt) needs to be updated.
Comment 6 Joel Rosdahl 2015-08-15 14:47:28 UTC
Hi again Mark,

Any progress on this patch?
Comment 7 Joel Rosdahl 2016-06-09 19:42:01 UTC
Closing this now since no progress has been made for a long time. Please create a new issue at https://github.com/ccache/ccache/issues if wanted.