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

CI: add Appveyor script #247

Closed
wants to merge 22 commits into from
Closed

CI: add Appveyor script #247

wants to merge 22 commits into from

Conversation

peterbud
Copy link
Contributor

@peterbud peterbud commented Apr 7, 2018

Currently not all test cases are running successfully, but we can monitor the build progress on Windows, see the results of unittests and test suites, and fix the remaining issues.

32-bit version: Build process example output
Unitests: FAILED: 3 tests
Test suits: base/CCACHE_NOHASHDIR fails and aborts

64-bit version: Build process example output
Unitests: fails pretty early at SUITE: argument_processing
Test suits: base/CCACHE_NOHASHDIR fails and aborts

jrosdahl and others added 15 commits February 17, 2018 10:51
* 3.4-maint:
  Hash preprocessed headers located in “.gch directories” correctly
* 3.4-maint:
  Fix paths to bundled zlib
* 3.4-maint:
  The Dockerfile was moved to a subdirectory
Having large precompiled header files slows down direct mode off
CCache considerably. Allow CCache to check against a (much smaller)
pre-computed checksum file instead of the precompiled header file
itself. This checksum file will be used in the manifest instead of
the actual precompiled header file.

Note that to make this to work, the build system needs to keep a
checksum file in sync with the precompiled header.

The behavior can be activated by the `pch_external_checksum` option.
This should make them nicely rendered in the GitHub file tree.
* 3.4-maint:
  Change extension of AsciiDoc files to .adoc
* 3.4-maint:
  cleanup: Remove special-casing of files from ccache 2.x
  Fix debug_prefix_map suite
  Improve AsciiDoc markup
  Improve man page generation
  Remove redundant .gitignore entries
* 3.4-maint:
  Update NEWS
  Clean up
  Support out-of-source builds
  Be consistent with “Clang” and “NVCC” names in NEWS
  Don’t apply Clang workaround for PCH dependencies for other compilers
* 3.4-maint:
  Clean up
  win32: Silence compiler warnings
  cleanup: Improve robustness when multiple cleanups run concurrently
  cleanup: Log size before cleaning
  Add x_try_unlink utility function
  test: Use helper functions instead of custom code
  test: Improve error messages from expect_{equal,different}_files
* 3.4-maint:
  Prepare for v3.4.2
  direct .i mode: Don't create tmp.cpp_stderr file at all
Currently not all test cases are running successfully,
but we can monitor the progress
@afbjorklund
Copy link
Contributor

Looks like a good start! Can you add a MSYS (Cygwin) build too, using the regular test framework ?

script:
    - ./autogen.sh
    - ./configure $HOST
    - make
    - make ${TEST:-test}

That platform has some quirks (missing symlinks, PE executable format) that would be nice to test...

@afbjorklund
Copy link
Contributor

Also, if you could rename it to .appveyor.xml it would "hide" the clutter.

The test part should probably be integrated/moved somewhere as well ?

@peterbud
Copy link
Contributor Author

peterbud commented Apr 8, 2018

For hiding the clutter: it's pretty common to hide CI related stuff in a separate subfolder, all CI frameworks can handle if files are in a specific subfolder in the source tree, example here

@afbjorklund
Copy link
Contributor

afbjorklund commented Apr 8, 2018

@peterbud : yeah, we have a .travis folder for that already.
And of course the good ole "misc" for stuff that didn't fit...

But I think only .appveyor.yml works right out-of-the-box ?
https://www.appveyor.com/docs/build-configuration/#yaml-file-alternative-naming

make unittest/run.exe
./unittest/run.exe -v && TRUE

../test/run -v && TRUE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely this whole block should just run make test, rather than hard-coding internal binaries and targets ?

appveyor.yml Outdated
MSYS2_ARCH: x86_64

build_script:
- C:\msys64\usr\bin\pacman --noconfirm --sync --refresh --refresh --sysupgrade --sysupgrade
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate --refresh ?

@peterbud
Copy link
Contributor Author

peterbud commented Apr 8, 2018

@afbjorklund : You can configure the location of the appveyor.xml when you enable Appveyor for a given Github repo:
image

That;s for example how I'm using it for darktable.

Let me know your preference, and I'll move the files there.

@afbjorklund
Copy link
Contributor

afbjorklund commented Apr 8, 2018

As for the CCACHE_NOHASHDIR and other Windows test failures, most have been fixed in 217a260
They are rmostly elated to paths, either the wrong delimiter (/ \) is being used - or it needs "escaping".

I will clean that up into individual commits - after we have gotten the MinGW and MSYS merged (#246).
Also seems like my autoconf check for GetFinalPathNameByHandle doesn't work properly anymore...

checking for GetFinalPathNameByHandleW... no

From a21da6f
The fallback code also had some stupid bug, that converted C:\Users to sers

@afbjorklund
Copy link
Contributor

afbjorklund commented Apr 8, 2018

Fixing Windows build is related to #122

This PR is the same as/replaces #69

@peterbud
Copy link
Contributor Author

peterbud commented Apr 8, 2018

@afbjorklund I have added MSYS to the CI matrix, based on your suggestions I have corrected the script. Also I have realized that you had in an earlier (not yet merged) PR #246 a solution for fixing log printing: I have taken that here as well to have the same code base.

@afbjorklund
Copy link
Contributor

Basically that log printing stuff is in every branch, guess we should just give up on "%z" and go for the portable printf stuff instead. But it's also annoying when you can't use all the regular things, of course.

@afbjorklund
Copy link
Contributor

I think this will be a good addition to the Travis builds, once those have been cleaned up...
Currently travis will only build the unittest, but not run it. But it might be able to in Wine.
I think some of the latest changes for MSYS/MinGW might actually have broken Windows,
but I suppose that can be cleaned up by adding some kind of "vanilla" Win build target too ?

i.e. without bash, with all the legacy DOS weirdness. and then just execute unittest/run.exe

Though that is a much bigger change, than "just" fixing the Cygwin and Windows build targets.

@peterbud
Copy link
Contributor Author

The MINGW32/MINGW64 builds are native builds, and if you run unittest/run.exe from the bash is the the same if you would run that from command prompt or from powershell. Even I believe we can add powershell script to the CI if you want, but in order to do that we need to make a proper deployment in the CI script, as the compiled executable has dependencies (MSYS2 runtime DLL and zlib) which needs to be copied next to the executable, as they are not on the PATH if running from a simple command prompt/powershell.

Let me know if you want to add a powershell/cmd prompt test part.

Also based on my tests some of the test cases are broken on Windows, but not too many, so with thorough work we can fix them one-by-one.

@afbjorklund
Copy link
Contributor

Just thought I saw some additional issues, when running it with Wine ? (plus some Wine bugs)

Anyway, the priorities are something like the following:

  1. Fix the Cygwin build, this should be pretty easy and goes into 3.4
    Mostly it is about missing symlinks, that funky .exe extension and the PE executable format...

  2. Try to address the issues running with MinGW (both 32 and 64)
    This might require some additional investigation of DOS quirks like paths and batch files etc.

  3. Perhaps look into supporting Windows as a real developer platform.
    This is the final and least likely option, since it requires a lot of changes. But the goal of Cleaning up the build system #169

jrosdahl and others added 5 commits April 23, 2018 22:04
* 3.4-maint:
  Use double when calculating cache thresholds
  Add a 32-bit build target, using multilib (-m32)
  Convert float config to double, add rounding
  Upgrade clang to 5.0, for the Travis docker
  Make sure to export ASAN_OPTIONS properly
  Add small helper to run all Travis tests
  Make sure to call configure with right path
  Add a travis-build container, for running locally
  Look for -fdebug-prefix-map feature explicitly
  Improve the Travis build matrix
  Fix log printing of before/after cleanup
  Remove irrelevant variable assignment in wipe_dir
  win32: Fix CCACHE_COMPILERCHECK=mtime test case
@peterbud
Copy link
Contributor Author

peterbud commented May 2, 2018

I have updated this to resolve the conflict to master, but I'm not convinced that I did it correctly. What is your preference with this PR?

@jrosdahl jrosdahl added os: windows Related to building or running on Windows portability Affects portability of building or using Ccache labels May 7, 2018
@jrosdahl
Copy link
Member

I'm going through old open PRs and have now come to this one.

I have updated this to resolve the conflict to master, but I'm not convinced that I did it correctly.

No, the diff doesn't look right. Also, it seems be based on 3.4-maint, not master.

What is your preference with this PR?

I don't know, frankly. I have now written a kind of brain dump on my view on Windows support here: #447.

@peterbud
Copy link
Contributor Author

Let's close it here. I'll open a new PR with an updated CI proposal

@peterbud peterbud closed this Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os: windows Related to building or running on Windows portability Affects portability of building or using Ccache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants