Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of precompiled headers #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

izmmisha
Copy link
Owner

Creating PCH file (/Yc option) works in usual way as any other object,
except that it will produce meta info (stdafx.pch.clcache file) contains
it's cache key as identification of headers files and it's contents.

Using PCH file (/Yu option) affects /showIncludes - it not contains PCH
includes, so for generating manifest hash additionally used PCH cache key.

So when PCH headers or it's content changed but no changes in source file
or it's headers then it will produce different cache entry.

@codecov-io
Copy link

codecov-io commented Aug 24, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@abe10f0). Click here to learn what that means.
The diff coverage is 89.74%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #3   +/-   ##
=========================================
  Coverage          ?   85.89%           
=========================================
  Files             ?        4           
  Lines             ?     1333           
  Branches          ?      206           
=========================================
  Hits              ?     1145           
  Misses            ?      137           
  Partials          ?       51
Impacted Files Coverage Δ
clcache/__main__.py 87.33% <89.74%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abe10f0...ab4ea6b. Read the comment docs.

Creating PCH file (/Yc option) works in usual way as any other object,
except that it will produce meta info (stdafx.pch.clcache file) contains
it's cache key as identification of headers files and it's contents.

Using PCH file (/Yu option) affects /showIncludes - it not contains PCH
includes, so for generating manifest hash additionally used PCH cache key.

So when PCH headers or it's content changed but no changes in source file
or it's headers then it will produce different cache entry.
@9a4gl
Copy link

9a4gl commented Jan 11, 2019

What is status of PR ? Does it work well or there are still issues or things to do ?
Is this suppose to solve only problem of genereting pch files or this will also allow you to use Use /Yu per project ? I haven't saw anywhere that clcache do not work with with /Yu but I have noticed if I enable precompiled headers per project it just do not cache, is it again due to some specifics on my setup or it is known not to work ?

@izmmisha
Copy link
Owner Author

I have noticed if I enable precompiled headers per project it just do not cache, is it again due to some specifics on my setup or it is known not to work ?

Yes, clcache doesn't support precompiled headers.

What is status of PR ? Does it work well or there are still issues or things to do ?

I have not tested it in real developing cycle.
Our projects require debug symbols, but using /Z7 is impossible because linking time is growing up to 30 min per project.

@9a4gl
Copy link

9a4gl commented Jan 11, 2019

What is your patch about, just generating pch files or to support /Yu in general ?

@izmmisha
Copy link
Owner Author

I'm sorry that not answered on your question before, I though it is obvious from the PR title.
Anyway this PR add support of caching even /Yu (/Yc) compiler flags used.

@9a4gl
Copy link

9a4gl commented Jan 11, 2019

I am always suspicious when title tells exactly what I want ;) Maybe I am reading too much newspapers... In that case I will cherry-pick your PR and try as I have strong feeling that dropping precompiled headers would be a problem in my development team.

@9a4gl
Copy link

9a4gl commented Feb 1, 2019

So, I was using your support for precompiled headers, it worked good so far, but today I got intro problems. Compiler started to report on few files that precompiled headers have /GT set but source/header does not. It is fact that few days ago I had /GT on some files that I removed. Is it possible that it match precompiled header file from cache but should create new one ? After I cleared cache everything compiled so I cannot reproduce it right now. I will try to reproduce it next week with minimal set of files.

@izmmisha
Copy link
Owner Author

izmmisha commented Feb 4, 2019

@9a4gl thanks for your feedback.
I'm looked at /GT option and also checked clcache code. This option should be included into cache key generation.

You mentioned that cleared cache, but do you tried (before clearing cache) clean build (rebuild or clean + build targets)? So probably with some bug with incremental build it wont to rebuild pch file with new options.

@9a4gl
Copy link

9a4gl commented Feb 4, 2019

It actually happened on my Jenkins test node, it does build using msbuild and it does /t:clean first. I have made minimal project and tried changing /GT option and I couldn't reproduce that.

@izmmisha
Copy link
Owner Author

izmmisha commented Feb 4, 2019

it does build using msbuild and it does /t:clean first.

Well, good point to start.

It actually happened on my Jenkins test node

Do you have log of build? probably it contains some useful info.

@9a4gl
Copy link

9a4gl commented Feb 4, 2019

Hm, my tests on minimal project were only with visual studio. Will try with msbuild. Unfortunately I I do not have log anymore :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants