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

Teach our Azure Pipeline to build and test also using MSVC #2098

Closed
dscho opened this issue Feb 27, 2019 · 5 comments
Closed

Teach our Azure Pipeline to build and test also using MSVC #2098

dscho opened this issue Feb 27, 2019 · 5 comments
Assignees
Milestone

Comments

@dscho
Copy link
Member

dscho commented Feb 27, 2019

We are very close to contributing Git for Windows' MSVC patches to the Git mailing list (gitgitgadget#149), and we should really have a test first, to make sure that we got it right, and also to keep it working once it is in core Git's master.

To that end, I already added a job that builds the vcpkg dependencies and offers them as a build artifact (no sense trying to rebuild those dependencies every single time we want to build & test Git...): https://dev.azure.com/git/git/_build?definitionId=9

@dscho dscho self-assigned this Feb 27, 2019
@PhilipOakley
Copy link

This has been sitting awhile in my todo/inbox. Finally got around to trying to get VS2017 installed and this checked (so I can do the #1848 et al 4Gb issue(s,s,s,s) looked at, hopefully with a bit of intellisense... 😉

However I got "invalid numeric argument '/Werror'" for fuzz-commit-graph.o, which I suspect is a new fault in the fuzz code..

phili@Philip-Win10 MINGW64 /usr/src/git (gfw-master)
$ make -j15 install
GIT_VERSION = 2.21.0.windows.1.80.gdc2ae9a382
[...]
phili@Philip-Win10 MINGW64 /usr/src/git (gfw-master)
$ make MSVC=1
Fetching vcpkg in C:\git-sdk-64\usr\src\git\compat\vcbuild\vcpkg
Cloning into 'vcpkg'...
remote: Enumerating objects: 43, done.
remote: Counting objects: 100% (43/43), done.
remote: Compressing objects: 100% (33/33), done.
remote: Total 57083 (delta 14), reused 31 (delta 10), pack-reused 57040
Receiving objects: 100% (57083/57083), 13.59 MiB | 1.67 MiB/s, done.
Resolving deltas: 100% (36731/36731), done.
Checking out files: 100% (3174/3174), done.
Building vcpkg

Building vcpkg.exe ...

  pch.cpp
[...]
Elapsed time for package curl:x64-windows: 1.4 min

Total elapsed time: 1.4 min

The package curl is compatible with built-in CMake targets:

    find_package(CURL REQUIRED)
    target_link_libraries(main PRIVATE ${CURL_LIBRARIES})
    target_include_directories(main PRIVATE ${CURL_INCLUDE_DIRS})

    Finished curl
    * new build flags
    CC fuzz-commit-graph.o
cl : Command line error D8021 : invalid numeric argument '/Werror'
make: *** [Makefile:2349: fuzz-commit-graph.o] Error 1

phili@Philip-Win10 MINGW64 /usr/src/git (gfw-master)
$ git status
On branch gfw-master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

phili@Philip-Win10 MINGW64 /usr/src/git (gfw-master)
$ git log -1
commit dc2ae9a38264da2af13b0f95cc4fa5eee2ebf893 (HEAD -> gfw-master, origin/master)
Merge: eb5d06f545 66af0b6eb8
Author: Johannes Schindelin <[email protected]>
Date:   Mon Mar 11 19:58:29 2019 +0100

    Merge pull request #2119 from dscho/update-stash-to-current

    Update the built-in `git stash` to the latest version

phili@Philip-Win10 MINGW64 /usr/src/git (gfw-master)

I was hoping to get to try out the new VS stuff, but I'm not there yet.

@PhilipOakley
Copy link

looks like this is a makefile error in that fuzz-commit-graph is not filtered out of L2349 (2nd line below)

$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
     $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<

where C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS)) so maybe this special case also needs filtered for the MSVC case (I think the fuzz compilation is noop/not-used for the linux case, and has a special/newish .PHONY: fuzz-all at the end of the Makefile).

@PhilipOakley
Copy link

the FUZZ_OBJ are defined at https://github.com/git-for-windows/git/blob/master/Makefile#L703-L710 but are then included in the OBJECTS list at https://github.com/git-for-windows/git/blob/master/Makefile#L2314-L2319 so will still need to resolve the MSVC compiler flags problem (eventually).

Eventually just went with make CFLAGS="" MSVC=1 just to get something compiling (in progress).
.. spotted some warnings

    LINK git-imap-send.exe
LINK : warning LNK4044: unrecognized option '/lnghttp2'; ignored
LINK : warning LNK4044: unrecognized option '/lidn2'; ignored
LINK : warning LNK4044: unrecognized option '/lgdi32'; ignored
LINK : warning LNK4044: unrecognized option '/lcrypt32'; ignored
LINK : warning LNK4044: unrecognized option '/lwldap32'; ignored
LINK : warning LNK4044: unrecognized option '/lws2_32'; ignored

so still steps to go.

@dscho
Copy link
Member Author

dscho commented Apr 4, 2019

Let's close this in favor of #2148

@dscho dscho closed this as completed Apr 4, 2019
@dscho
Copy link
Member Author

dscho commented Apr 4, 2019

However I got "invalid numeric argument '/Werror'" for fuzz-commit-graph.o, which I suspect is a new fault in the fuzz code..

Yeah, I forgot to take care of DEVELOPER=1. Sorry for that... See #2148 for details how I addressed this.

looks like this is a makefile error in that fuzz-commit-graph is not filtered out

No, it needs to be compiled in...

LINK git-imap-send.exe

LINK : warning LNK4044: unrecognized option '/lnghttp2'; ignored
LINK : warning LNK4044: unrecognized option '/lidn2'; ignored
LINK : warning LNK4044: unrecognized option '/lgdi32'; ignored
LINK : warning LNK4044: unrecognized option '/lcrypt32'; ignored
LINK : warning LNK4044: unrecognized option '/lwldap32'; ignored
LINK : warning LNK4044: unrecognized option '/lws2_32'; ignored

I addressed that as part of the commit that adds support for MSVC=1 DEVELOPER=1.

@dscho dscho added this to the v2.21.0(2) milestone Apr 5, 2019
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

No branches or pull requests

2 participants