Fix VS2017-YASM integration(#229), msbuild.bat VS2017 path(#226)

[KevinHake]

Merged from https://github.com/BrianGladman/mpir thanks Brian!

[KevinHake]

Almost completely grabbed out of @BrianGladman 's repo, this is exclusively for the Visual Studio build process. Brian you can diff against your repo to see what all I left behind.

The build.vc dirs were moved into msvc, and we will be supporting VS2013, 2015, and 2017. I kept the mpir.texi changes related to the VS build. Minor change to mpir_config.py to make it callable from anywhere (couldn't use it inside the msvc dir before with local path). The .gitignore is updated with the tons of potential VS build junk.

[BrianGladman]

I am sorry, Kevin, but I am not happy to see changes for the Windows build process being merged into the MPIR master repository by any other method than a direct merge from my repository. If this is done in any other way it would mean that I am no longer in control of the MPIR Windows build process.

[KevinHake]

@BrianGladman I hear you, you put in the effort to arrange this and I, and many others (@SanderBouwhuis, @rohan-shah, @mmcs85, @madscientist to name just a handful) definitely appreciate being able to compile this with the latest VisualStudio. The last thing I am looking to do is reinvent your build process that is working well for me, or take the credit. This isn't different from your repo - if you do a KDiff3 with this branch, wbhart:master, and your own master, you will find this is your change. If you're worried about the credit, please by all means upload it on your own, and we can ignore this one.

Here's where I am coming from: I need to be using the official, tested repo for what I'm working on. I know I'm not the only person with this requirement (@dlaugt for one), and it's good practice to upload improvements as they are made. 6 months ago I was happy to be able to use your solution, even if I had to pull it into wbhart:master on my own. I think it is a shame to have the solution ready for so long and not bring it in here where the people who need a tested codebase can use it. I really want your work to see the light of day.

To keep this project alive, reviews need to happen. I was thinking that you were too busy to do the pull request, and I wanted to do my part to help keep the project going. I think @wbhart will agree that it is hard to get your changes into the official repo without creating pull requests for those changes.

I also believe that taking part in reviews is a great way to stay in control of the build process while leveraging the time and effort donated by other interested devs. I have seen a number of references in the forums and comments to MPIR being shorthanded right now, and it's such a great project I'd love to see it gain some needed attention and grow.

I really hope I haven't upset you @BrianGladman, I do appreciate your support and I look forward to seeing more of your contributions. Please let me know what can be done to expedite getting this change in.

[KevinHake]

@isuruf - I added the image: Visual Studio 2017 line, and also moved the msbuild tests to the front so we don't need to wait so long to see it fail :)

[KevinHake]

@isuruf on my local machine, the msvc/vc17/msbuild.bat fails when it tries to discover the path to MSBuild. It looks like we are using the snippet from here: https://github.com/Microsoft/vswhere/wiki/Find-MSBuild ...but my testing shows that it just doesn't work. (To clarify, microsoft's example doesn't work. And from my testing, vswhere in general fails to actually report the locations of half of my VS installs on my machine). Taking a look at the history, this was updated in @BrianGladman 's very last commit (475408d), which I believe is meant to fix #226.

I think for now I'll revert just that one change, and we ought to have a working build that at least solves #229 Edit: It was not working locally because I'm using a Preview version, and as written the script won't return a path for such. Looking at the Appveyor log, it does get past this thanks to the image: Visual Studio 2017 line.

So it fails to compile: c1 : fatal error C1083: Cannot open source file: '......\mpf\cmp_z.c': No such file or directory [C:\projects\mpir\msvc\vs17\lib_mpir_gc\lib_mpir_gc.vcxproj] I'm looking into it.

[BrianGladman]

The msbuild.bat file will need to be modified to add the windows SDK to the msbuild call because we are all using different versions of Visual Studio, which is why the batch file is failing for some and not for others.

The file is also faulty in detecting errors in the inputs so it will need more work before it is fit to go into the main repository.

[SanderBouwhuis]

@BrianGladman That would be great! Now, after I download your source code I search/replace all the SDK versions in the .vcxproj files.

[BrianGladman]

I have now updated msbuld.bat to take an additional parameter to set the Windows SDK version. It is for VS17 only and is now in my repository. Please give it a try and let me know if you have any problems.

[SanderBouwhuis]

I will wait for the dust to settle on the new 'MPIR for Windows' and then I'll try the complete thing. Thanks for all the effort you guys in putting in this.

[KevinHake]

@BrianGladman - Can you explain to me more about the problem your msbuild.bat file is addressing? Is there a GitHub issue number for it? I don't know in what case a build requires the SDK version to be set explicitly.

Side note - I am able to get msbuild.bat to run fine on my machine if I just add the -prerelease flag to the vswhere call (since I'm using a VS preview version, and by default vswhere omits all prerelease info).

[SanderBouwhuis]

You have to specify which SDK you are targeting. It has to be the same for the project you are including it to. This is because the run-time libraries have to be the same. In the *.vcxproj file you can specify this by this line:

<WindowsTargetPlatformVersion>10.0.17134.0</WindowsTargetPlatformVersion>

[BrianGladman]

@KevinHake As @SanderBouwhuis says, if MPIR forms part of a larger application, you have to ensure that both MPIR and this application are built to use the same version of the Microsoft run-time libraries. The reason you don't necessarily have to set the SDK version is that I set an appropriate default when the MPIR build files are generated by mpir_connfig.py. But this is not always helpful since it turns out that those of us who are developing and/or using the development versions of MPIR are all using different versions of Visual Studio 2017 which bring with them different SDK versions. As a result I keep getting reports that msbuild.bat doesn't work when in fact it works fine but fails to build because the default SDK set in the build and that used as default by Visual Studio are different.

All of which makes it desirable that users have the ability to set the Windows SDK version they wish to employ in an MPIR build.

[KevinHake]

@SanderBouwhuis & @BrianGladman - thanks for the explanation, it makes total sense to me now. The only issue I have with the new msbuild.bat is that right now if I give it the same command line that I used to (which someone who isn't aware of the change is apt to do), for example msbuild.bat gc lib x64 Release +tests, it will use the +tests string as the SDK version, and will not build tests. Ideally the SDK version and test flag would both be optional, which would require a little more argument parsing code for the last args; if the 5th arg is +tests, then build with tests and use the default SDK. If not use it as the SDK version, and check the 6th arg for the +tests flag.

[BrianGladman]

@KevinHake I'll see what is possible on making the two last parameters optional. Sadly it is command line script and is quite a pain to get right.

EDIT: I have just updated to make the last two parameters optional.

[KevinHake]

@BrianGladman I tested the msbuild.bat - I couldn't come up with a good way to verify that the binaries generated actually are targeting the version I say to use, but it appears to be working, and the two optional params are perfectly optional 👍 I added it in here to this branch.

I think I also found what may have been the last bug making the AppVeyor run fail (I clobbered perfpow along with zero_p and cmp_z, and I shouldn't have). If the checks pass, this should be a faithful merge of all your windows build changes.

[KevinHake]

@BrianGladman - Looks like it's working now. I have a couple more questions after doing a grep for "build.vc"...

• Is "mpir.net/" still used?
• Is msvc/g2y.py still used/relevant?
• There are msvc/run-tests.py and msvc/vs1/run-tests.py... should msvc/run-tests.py still exist?

[BrianGladman]

@KevinHake Thanks for tidying up some of the incorrect references to the old build directories.

I am not involved in the mpir.net component of MPIR so I can't say much about it.

msvc/g2y.py is a development only tool that I use when I need to convert GCC assembler code into Intel syntax assembler code. I still need this in my repository but it is not needed elsewhere.

The duplication of run-tests.py in msvc and in vs\mpir-tests is the result of work in progress. I would like to be able to have run-tests.py in msvc only and run it on the last built tests but this requires changes to the output_params.bat file format that I have not completed.

[KevinHake]

@BrianGladman & @wbhart - I think this is ready to be merged. It captures all of Brian's build improvements and fixes. The MSVC build is more tidy (a lot of the auto-generated files have been cleaned up, and all the build files are in a single dir now so it's out of the way for non-msvc folks). Brian's remaining additions should be much easier to review with these changes out of the way. And most importantly, this will give many more people the ability to build for Windows.

[BrianGladman]

@KevinHake We are currently held up by three issues on the 'brian_master' pull request I'm afraid.

[KevinHake]

@BrianGladman - I would argue that this branch functions as we want it to, fixes many problems, and is not held up by anything - there are no merge conflicts and the builds all work.

One of the issues I see with merging in brian_master is that right now there are no windows build files in master. Most of the changes we want to bring in from brian_master are dependent on those build files, and this branch has them all, ready to go.

Once this is in, I think the other merges become easier. I am doing my best to break these merges up into modular pieces so we can have incremental improvements instead of giant monolithic changes that will be ready "one day". @wbhart what's your opinion?

Also I am pretty familiar with git these days and I'm happy to help with your remaining merges @BrianGladman , as well as getting you set up for a nicer workflow going forward. Side note - I would also like to pick your brain about number theory some time :) Maybe at some point we can have a live chat for a git - number theory exchange.

[SanderBouwhuis]

If you guys want someone to try this out, let me know. Any problems/hiccups I encounter I'll give feedback on. What is the ETA for a 'complete' release?

[BrianGladman]

@KevinHake The problem here is that you want to merge the new Windows build system before my other changes whereas I want to do this in the reverse order. This is not a technical issue but a political one and as such it may well prove to be a great deal harder (or even impossible) to solve.

The changes I want to make to the MPIR source code are large when judged on the source files impacted only because I have removed the outdated GMP_PROTO macros needed to deal with ancient C compilers. Other than this, the changes are modest and amount to the addition of three new functions that GMP has added which we do not currently have. To add these requires very minor additions to the Linux/GCC build system and it is the lack of effort on this that is holding us up.

This is emphatically NOT a big job so the effort is not large and our ability to get this work done is a clear test of what we have - i.e. do we have one MPIR that supports both Linux/GCC and Microsoft/MSVC or do we have two separate MPIR versions, one for Linux/GCC and one for windows/MSVC? Our ability to merge my non-Windows specific changes into MPIR answers this question and this is why I want it merged first.

There is then a second issue that reflects on continuing with one MPIR rather than two, which is whether it should be possible for Windows/MSVC users to build MPIR 'out of the box' - i..e. by immediately loading a mpir.sln file into Visual Studio. This has always been possible since we forked MPIR and i am unwilling to drop this completely as a feature. But this requires that an MPIR distribution would include some now auto-generated files. I have a lot of sympathy with the desire to cut down the size of the distribution and I have hence completely reworked the Windows build and cut the number of pre-built Windows builds down to just three - a generic C DLL and C and C++ static libraries. But I am reluctant to go further since I know from past experience that that when people find that they can't build MPIR with Visual Studio 'out of the box' I will get a lot of mail asking how to build MPIR on Windows (saying RTFM doesn't change the amount of email I will get).

Right now we do not have an agreement on the way ahead here. As far as I can tell Linux/GCC folk are unwilling to accept that an MPIR distribution should include even the minimum three 'out of the box' builds that I need; and I am unwilling to accept that an MPIR distribution won't include the ability to build 'out of the box' with Visual Studio. This is unresolved.

So in summary, the reality is that we now have two versions of MPIR - one for Linux/GCC and another for Windows/MSVC. We are trying to produce one MPIR covering both environments but it remains uncertain that this is an achievable goal.

[KevinHake]

@SanderBouwhuis - I intended for this branch to be purely the windows msvc build changes implemented by Brian, separated from any other functionality so it could be easily merged with the stable repo. A few minor script changes were also made to get the automated tests running. For me it works, and the automation works, so I suspect you will have no problems. If you have the time, I certainly won't object to hearing your results.

[KevinHake]

Has anyone pulled this down, and used a real, non-web-based diff tool to compare this branch to wbhart master and review it properly? If it seems too much, talk to me because I really think it's fairly simple and should only take 5-10 min to see what's going on.

@BrianGladman Philosophy: Should Windows and Linux builds live together? MPIR is already well known for being a GMP port that is compilable in Windows. If someone only wanted a pure linux app, as a newbie, I would choose GMP. Personally I use both platforms, hence I chose MPIR. I suspect many large distributed apps with a need for multiprecision arithmetic choose MPIR precisely because it is multiplatform, and they know they can use it on Windows and Linux with few porting issues. I think separating the builds weakens what makes MPIR special.

@wbhart - the msvc Windows build takes up 25% of the repo here. I think it's a lot. This branch is a big improvement over previous builds (that built with msvc), and we can reduce it more as we go (perhaps the test projects are a place to look). Tho big, it's now in one dir, I can clone the entire repo in under a minute in the boondocks of Mexico. I'd like to keep development rolling incrementally. Do you really have any objections about merging in this branch? I think further improvements can be addressed under separate issues.

Why should this be merged before the rest of Brian's changes?

1. MSVC is unquestionably the most popular compiler on Windows, and functions well. Right now master won't compile with it, this branch does, properly.
2. It is ready, now.
3. It makes the rest of Brian's unmerged changes easier to merge; Much of the rest of Brian's changes are dependent on changes to the windows build which are encapsulated here. Trying to merge those first will be an exercise in duplicating this branch. It will be difficult to discuss Brian's other changes clearly while they are all mixed up with large build system refactor commits.

I don't see the downsides to merging this now, political crisis or not. It has minimal impact on pure Linux folks. It will please a number of people with open issues as well as anyone who wants to build with visual studio 2017. It will expedite and clarify the discussion of philosophy/politics and Brian's other changes.

Brian if you have doubts about this being a proper merge or your build improvements, or that it will simplify your existing merge dilemma, we can go through it together.

[wbhart]

Kevin, I can't review a PR that changes 1885 files! I've spelled this out in my responses on these threads. The entire merge needs to be done as a set of human reviewable PR's, and we need to take steps to preventing things like this happening again. Git is currently being used improperly, and I especially do not have time to sort it out. This has been driving experienced contributors away from the project.

I appreciate that you have put time into this, and are keen to see the job completed. But Brian must be able to approve of and understand all changes to the Windows build so that he can continue with his work on it. Sidestepping his input is not the right way forward.

Currently we are waiting on someone to help with changes necessary for the Linux build. This is a result of a review in progress. Someone has apparently agreed to undertake that work. When it is completed, we will move on to the next step.

[KevinHake]

@wbhart - my apologies, I just wanted to communicate what's in this branch, and I think I'm not doing a great job... You're right we did talk about the numbers before. Almost all are directory renames and deletions (my second comment in the thread explains). I know Windows build is not your realm, and I definitely don't want to be bypassing Brian. I was just intending to address Brian's point that there was dissent about keeping any Visual Studio project files in the repo - I thought it was a reference to you. I wanted to know if that's still a barrier.

Anyway, I have what I need, and it's here for you guys to decide if it's helpful for you too. Hopefully I have been more catalyst than nuisance! For reference if someone does feel inclined to have a look, it is easier to see the refactor structure by diffing against the commit before the windows build files were removed (90740d8f), but of course Brian will know what's what either way.

[BrianGladman]

@wbhart With reference to improper use of GIT, "I plead guilty, me lud'. I am not a confident user of GIT and I don't trust myself to use it properly because whenever I attempt to do so, I make mistakes and things go very badly wrong and I then have considerable difficulty in recovering. I willingly admit that this is all my fault in not knowing how to use GIT properly but my life is complicated enough already with a mass of things I still need to learn and, as a lone developer, I don't have colleagues around from whom I can easily pick this sort of thing up without too much effort.

In my defence, I would say that I have been working in the way I am now ever since we launched MPIR and it has not caused problems until now so its not obvious to me why anything I have recently done has caused the current difficulties. I think the issue is much more the result of an almost complete lack of collective MPIR effort over the last couple of years since you moved onto other things.

This is well illustrated by the current situation where we are held up for what I believe is a minor amount of effort from Linux/GCC folk in finding out why my changes to the MPIR source code cause Linux/GCC build failures.

[BrianGladman]

The issue of the 'cost' of MSVC support within an MPIR distribution has been an emotive one and may have been confused with a desire for purity in that repositories are believed by some to be places where all auto-generated files should be banned.

I frankly don't care what does and does not go into the MPIR master repository in this respect but I do care about maintaining the ability of Visual Studio users to be able to build directly 'out of the box' from my repository and from any MPIR distribution.

The cost of now doing this in terms of impact on the size of an MPIR distribution prior to compression is 204KB. If this is really such a problem, there are minor changes to the MPIR source code (that all test files in the tests sub-directory that result in executable tests have names starting with 't-') that will allow me with relatively little effort to reduce the size of a distribution by some 20MB.

So if the objective is to cut the size of the distribution, we are looking in the wrong place.

[BrianGladman]

@KevinHake I agree wholeheartedly with you that a major benefit of MPIR is for the large number of users who offer multi-platform applications built with both Linux/GCC and Windows/MSVC. I don't want to lose this but neither do I want to pretend that we have an MPIR that achieves then when in practice we don't. And, without a willingness on both sides of the community to put in some collective effort towards this aim, we don't.

You mentioned that you use both LInux and Windows. So, if you really want to make progress, you could volunteer to find out why my changes cause the Linux/GCC build to fail. Indeed, if you find the changes in my repository that result in good Linux/GCC and WIndows/MSVC builds, we will be able to get past the current hold-up.

[wbhart]

This is now no longer needed. Thanks for the effort that was expended on this.