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

Make the tests work under Windows #943

Closed
wants to merge 3 commits into from

Conversation

rvogg
Copy link
Contributor

@rvogg rvogg commented Oct 6, 2021

With these changes I made some adaptations that the tests run under Windows again.
Tests that fail and cannot be fixed quickly have been deactivated for the moment.
Additionally I added mingw-w64 builds under Windows, because the msvc builds contain other bugs.

In some places I had to give the shell scripts a '.sh' extension to make them run under Windows. It looks like this does not affect the other operating systems.
( CCACHE_DETECT_SHEBANG does not work correctly -> The tests produce a "could_not_find_compiler" error if the .sh extension is missing.)

I hope this change helps avoid breaking existing behavior on Windows and make ccache even better on Windows.

Note: The secondary_http test fails sporadically on Windows. (Perhaps it should also be disabled)

@louiscaron
Copy link
Contributor

louiscaron commented Oct 19, 2021

This looks good to me. Actually, i had to implement some very small portions of it in order to implement and run on Windows the testcases for the new compiler I would like to add support for (#942).

If, by chance, your patch gets merged soon I would rebase my own patch. thanks

@rvogg rvogg force-pushed the fix-windows-tests branch 2 times, most recently from 5e61de3 to 473f583 Compare October 26, 2021 20:01
test/suites/modules.bash Outdated Show resolved Hide resolved
@rvogg rvogg mentioned this pull request Nov 2, 2021
6 tasks
@jrosdahl
Copy link
Member

jrosdahl commented Nov 3, 2021

With these changes I made some adaptations that the tests run under Windows again.

Note that the Windows tests are work in progress and have never worked before, so it's not the case that they have become broken.

@jrosdahl jrosdahl added improvement Improvement that is not a bug fix or new feature test Affects testing of Ccache itself labels Nov 3, 2021
@rvogg
Copy link
Contributor Author

rvogg commented Nov 3, 2021

With these changes I made some adaptations that the tests run under Windows again.

Note that the Windows tests are work in progress and have never worked before, so it's not the case that they have become broken.

I thought that because there were many if ! $HOST_OS_WINDOWS are spread over the tests, it was less broken than now.
Currently not a single test runs on Windows because they all fail on the expect_stat function.

My initial motivation for this change was: I started to fix the basedir functionality for Windows, because it doesn't really work with all the Windows path mess.
After a few #ifdef __Win32 changes I realized that I broke a lot more but couldn't really check what. So I first tried to get to a state where I could verify my changes.

I didn't fix all tests, because in some cases ccache really doesn't work on Windows or the effort to get it running in the git-bash was too high. And I don't expect to fix all bugs for ccache under Windows, but only that no additional bugs get in by mistakes.

@rvogg
Copy link
Contributor Author

rvogg commented Nov 9, 2021

I added the commit 'handle newlines on Windows properly' from #954. With this I was able to remove some hacks.

I did not add the commit 'handle properly also stdout in addition to stderr' because it breaks the correct ccache behavior.
With export CCACHE_NOCPP2=1 the preprocessor output is written to stdout.

- Adding the ".sh" extension to most shell scripts.
  It looks like Windows needs this to run the scripts.

- Adding special handling of carriage return characters.

- Tests that fail are deactivated for the moment.
@jrosdahl jrosdahl changed the title Made the tests work again under Windows. Made the tests work again under Windows Dec 14, 2021
@jrosdahl jrosdahl changed the title Made the tests work again under Windows Make the tests work under Windows Dec 14, 2021
@jrosdahl
Copy link
Member

@rvogg wrote:

I thought that because there were many if ! $HOST_OS_WINDOWS are spread over the tests

I understand. @nickhutchinson made several Windows-related improvements (#757, #780, #789, maybe more) of the test suite, but it was never enabled in CI so I don't think it worked fully.

My initial motivation for this change was: I started to fix the basedir functionality for Windows, because it doesn't really work with all the Windows path mess.

Regarding that, I think that the way to make basedir work in a reasonable cross-platform way is to start using std::filesystem::path internally instead of handling paths and normalization spread throughout the code. See also #866.

@orgads
Copy link
Contributor

orgads commented Aug 14, 2022

@rvogg ping

@orgads
Copy link
Contributor

orgads commented Aug 14, 2022

Rebased and created a new PR #1133

@rvogg rvogg closed this Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement that is not a bug fix or new feature test Affects testing of Ccache itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants