Add source code (not build) changes that affect both Linux and Windows

[BrianGladman]

The only new files in this pull request are (1) mpf\cmp_z.c, (2) mpn\generic\zero_p.c, and (3) tests\mpn\t-logic.c. I have not amended the Linux/GCC build system to add these files.

Edit: I did attempt to modify the makefile.am file in tests\mpn but the name I added (logic.c) should be t-logic.c

Edit 2: changed mpn\zero_p.c to mpn\generic\zero_p.c - I have done (1) but not (2) and (3)

[BrianGladman]

Looking at the appveyor and Travis Cl integration failures, these are because my mod to tests\mpn\makefile.am used the name logic.c rather than t-logic.c

The results look encouraging - correcting this error and adding the two new files I mention above will hopefully give a good build.

[wbhart]

I still can't review the changes here. It shows 1339 files have been altered. Pull requests are not able to be reviewed if they touch so many files.

I think this is because of all the files you deleted. That has to be done as a separate PR, I'm afraid.

You can do that on a separate branch. Just fetch from me and create a branch based on my master, delete all the files you want deleted, then issue a PR from that branch. After I've merged that, this PR will be readable (you won't have to redo it!)

[KevinHake]

@wbhart, @BrianGladman - This is a monster of a merge on the surface, but let me float some ideas to you guys about ways to make it doable.

What makes it hard right now is that a lot of changes have all gone into Brian's master branch, from different topics (some windows build, some general code improvements, etc) and they're all mixed together over a long time period. Mostly they're independent, but not entirely (I'll get to that). Hindsight is 20/20, and it would be a lot easier to merge if incremental changes to the windows build went in one local branch, linux-only in another, etc, with periodic pull-requests to merge those back to the wbhart:master... we can do that for future changes, but here's how I think we can gather what we have without too much headache and without any loss.

This particular pull-request has a lot of the windows build refactor commits in it, and I think ideally that would be done separately... As mentioned earlier, we could make separate branches in Brian's repo for windows build, code changes, etc, then take all the commits he has in trunk, and cherry pick each one into those new branches. I personally think this is too hard, for one because I think it almost certainly requires at least a screen share and potentially hours. Not to mention we might not care about all of the history of those changes (e.g. I know for me I often have several "oh one more thing I forgot" commits, or "fixing a typo" commits), and so getting bogged down reviewing each thing might not be worth our time.

At this stage I think it may be easier to manage what changes to bring in by ignoring the history of commits, and just taking a look at a top-level directory diff (I'm a big fan of kdiff3 for this) of Brian's latest master vs wbhart:master. From there it may be easier to create a second, fresh mpir clone, and in respective branches for windows build, code changes, etc, re-construct all of those changes in a relatively small number of clean commits. The end result being a new cloned repo, with all of Brian's changes, but in just a few commits, and in a form that will easily merge (if not merge automatically). The more messy old clone can then be renamed and kept around as an archive of the dev history that produced the end result. When all the merging is done, wbhart:master should have a handful of new, readable/reviewable new commits of Brian's changes, and we will be able to verify again with a top-level directory diff that indeed all of the needed changes were transferred from Brian's old repo into the main trunk.

This top-level diff and creation of "summary" commits is the method I used to extract only Brian's Windows build changes into the branch I put up for review. I think it is very close to being totally separated out, but I believe some of the new VisualStudio build files are dependent on some of Brian's new application code (code that will be used in both Linux and Windows); that is, they are trying to build them. This particular part of the merge is the one producing the hundreds of file changes statistic - there are 1774 windows build files. With Brian's refactor, we drop support for VS2012 (deleting 322 files), and the rest are moved/renamed to be in one single directory (plus removal of project files that can be auto-generated with a config script). Tho certainly not reviewable on the github site (which has a very limited diff tool), it is straightforward with any popular directory diff tool. After the windows build refactor, you will find the remaining touched files are very manageable to review, maybe even in the github web interface.

[BrianGladman]

@wbhart I wrongly assumed that it was my GMP_PROTO removal that was causing the huge number of changes but this is not as large as I thought. It seems that my pull request may remove all my old Windows build files, although this was not my intention. These will need to be deleted soon but I was trying to leave these alone at this point and do only those source code changes common to both Linux/GCC and Windows/MSVC.

[wbhart]

OK, I just learned more about GitHub's diff than I'd like to know. Apparently it does not diff between the PR and the latest HEAD. It diffs with the last common ancestor. This means that no amount of pull requests is going to get us a clean diff.

This means that we have to do a rebase. Someone is going to have to rebase Brian's mscv_build branch on my latest master. I doubt this can be done with the graphical interface due to the deletion of files. I think the only way to do it is from the command line interface.

Sorry about this, but I was unaware of this issue until just now.

The good news is the latest changes do seem to make the Linux build pass!

[BrianGladman]

@wbhart Bill, I need to add the file mpn\generic\zero_p.c to the Linux/GCC build to complete it - can you please advise where I have to add this?

I don't understand the issue you have just raised. Is this an issue with this pull request? It is the reference to my temporary parking of the new msvc build in my msvc_build branch that is confusing me.

If it is a diff you need so that you cn review the changes, I could generate a diff between your master and mine using Beyond Compare (my file/directory compare app) if that would help. I am somewhat scared of rebasing since I spent much of yesterday trying to do this without success and it kept messing up my local repository.

[wbhart]

@BrianGladman Sorry I meant master, not msvc_build.

The change you want to make should go in master, and someone is going to have to rebase your master on mine.

[wbhart]

@BrianGladman I took your master and merged it with mine and then made a PR based on that [1]. This reduces the "diff" to changes to just over 100 files, which is a bit more manageable.

I think it will be possible to review it in this form.

I'll wait until you've added the change you want to make, to your master, and I'll add that to the new PR I made. Then I'll begin reviewing that PR.

This means we don't need to do a rebase. However, you are probably going to have to remove your master branch and create it afresh from mine, otherwise we will forever have dozens of additional commits showing up every time you do a PR.

In future, I strongly recommend working in a branch other than master, doing regular merges from my repository into your master, then rebasing your branches on your master before making regular human readable PR's from whatever the current new branch is that you are working in. (So I'm advocating you have two working branches at any time, master, which only ever receives updates from my repo, and whatever the current branch is you are working on. Once the PR is accepted for that branch, delete that branch and create a new branch to start working on the next feature.)

The autogenerated files probably shouldn't be in the repository at all. They probably should be added only when we do releases (both for Linux and for Windows, in my opinion). There's simply no way anyone can review PR's when hundreds or thousands of files are changing in tens of thousands of places.

[1] https://github.com/wbhart/mpir/pull/258/

[BrianGladman]

@wbhart Thanks Bill, I was dreading yet more time on rebasing.

In respect of my current master branch and the current pull request the only change needed is the addition in the Linux build system to add the file mpn\generic\zero_p.c, which I don't know how to do.

I also need to merge my msvc_build branch back into my master to add the new MSVC build and then get this into your repository. I was intending to do this with a new pull request after we have completed the current one but I can do this anytime that you think is best.

After we get these two things done, I am more than happy to delete everything and restart based on your repository! The sooner the better as there are minor but important updates that are needed in the new Windows build that I would like to get back to!

I understand your concern about auto-generated files in the repository but many people on Windows expect to be able to build MPIR in Visual Studio 'out of the box' without having to go through additional hoops. So what I have done at the moment is to cut the 'out of the box' Visual Studio Build files down to three (sadly for each of three versions of VS) - the MPIR generic C static and dynamic libraries and the C++ library. I could drop this for the two older versions of Visual Studio to get it down to 3 total and, as you say, we could drop these completely if we put them only in a distribution. But we don't do distributions often these days.

[wbhart]

@BrianGladman I don't think it will be possible to merge anything back into your current master. So long as your MSVC changes are in some other branch, the work won't be lost. Unfortunately, I don't think you will be able to keep the commit history for that branch though. But we will cross that bridge when we come to it.

As for adding mpn files, there is documentation in the devel directory on how to do this. Changes need to be made in quite a few places.

If you are not up to it, we need to find someone on the Linux side to take care of it. But we aren't going to find someone to do that until I've merged the big PR. And that won't happen until I find time to review the big PR.

Since it is such a large code dump, it is going to take me some time to work through it, and I have a job to get on with right now. So there's simply going to be a delay until I can get to this.

[wbhart]

I'm going to close this PR, and focus exclusively on the new PR I created from your master.