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

Fix and simplify handling of options for MSVC #954

Merged
merged 1 commit into from Dec 12, 2021

Conversation

llunak
Copy link
Contributor

@llunak llunak commented Nov 1, 2021

These are various fixes (on top) of #506 .

@llunak llunak force-pushed the msvc-fixes branch 3 times, most recently from 85ee077 to be8e770 Compare November 2, 2021 11:54
@llunak llunak mentioned this pull request Nov 2, 2021
@jrosdahl
Copy link
Member

jrosdahl commented Nov 4, 2021

I'd appreciate it if you could submit "handle newlines on Windows properly" and "handle properly also stdout in addition to stderr" as separate PRs since they are independent.

@llunak
Copy link
Contributor Author

llunak commented Nov 4, 2021

I'd appreciate it if you could submit "handle newlines on Windows properly" and "handle properly also stdout in addition to stderr" as separate PRs since they are independent.

They are not, they both modify the block in ccache.cpp around line 950.

@jrosdahl jrosdahl added this to the 4.6 milestone Nov 6, 2021
@jrosdahl
Copy link
Member

jrosdahl commented Nov 7, 2021

They are not, they both modify the block in ccache.cpp around line 950.

I did not mean that the commits as currently written are independent, I meant that the fix and the feature are logically independent of "MSVC fixes". The first can be introduced separately to fix send_to_stderr on Windows. This point is quite minor and I'm OK with not doing that one separately if you think it's hard or tedious to do so. The second one however is a generic "we can now cache stdout for any compiler" feature, not an MSVC fix per se.

@rvogg
Copy link
Contributor

rvogg commented Nov 9, 2021

The commit 60a6d2d breaks the correct behavior of ccache.
With export CCACHE_NOCPP2=1 the preprocessor output is written to stdout.

@llunak
Copy link
Contributor Author

llunak commented Nov 14, 2021

I did not mean that the commits as currently written are independent, I meant that the fix and the feature are logically independent of "MSVC fixes". The first can be introduced separately to fix send_to_stderr on Windows. This point is quite minor and I'm OK with not doing that one separately if you think it's hard or tedious to do so. The second one however is a generic "we can now cache stdout for any compiler" feature, not an MSVC fix per se.

Done in #962.

The commit 60a6d2d breaks the correct behavior of ccache. With export CCACHE_NOCPP2=1 the preprocessor output is written to stdout.

Fixed there.

Since MSVC understands both /option and -option, change all of them
to -option, which means now old code handling e.g. -U gets reused,
and this also simplifies handling of both variants (/Fo was handled,
but -Fo wasn't).

The list had two options -P and -u that mean something else for MSVC
than it does for GCC, so handle those explicitly.
@llunak llunak force-pushed the msvc-fixes branch 2 times, most recently from e03c163 to 153bae1 Compare November 28, 2021 17:23
@llunak llunak changed the title MSVC fixes Fix and simplify handling of options for MSVC Nov 28, 2021
@llunak
Copy link
Contributor Author

llunak commented Nov 28, 2021

I've separated these commits into more PRs, so this one is now only one commit.

It makes the code properly handle both -option and /option way of writing MSVC options, which also fixes several places where they weren't both listed properly. It is done by converting all /option arguments to -option (rather than listing both cases everywhere and risking mistakes), and if the handling gets to passing the argument as a file it gets changed back.

@jrosdahl jrosdahl added the compiler: msvc Related to Microsoft Visual C++ label Dec 12, 2021
@jrosdahl jrosdahl merged commit 0a644e6 into ccache:master Dec 12, 2021
@jrosdahl jrosdahl added the bug Does not work as intended/documented label Dec 12, 2021
@jrosdahl jrosdahl mentioned this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Does not work as intended/documented compiler: msvc Related to Microsoft Visual C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants