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

CMake build system for git #614

Closed
wants to merge 33 commits into from

Conversation

SibiSiddharthan
Copy link

@SibiSiddharthan SibiSiddharthan commented Apr 22, 2020

This is an attempt to build Git using CMake. CMake is cross-platform build generator which works well on a variety of platforms(primarily Linux and Windows). Using CMake we can check whether
certain headers exist, certain functions exist, the required libraries are present and configure the build accordingly. Using CMake we can also build and test Git out of source, resulting in a clean source tree.

Tested platforms

Ubuntu 18.04
GCC 7.4
Clang 8.0.1

Windows
MinGW GCC 9.2
Clang 9
Visual Studio 2015,2017,2019

Changes:

  1. The CMake script has been relocated to contrib/buildsystems
  2. The CMake script parses the Makefile for the sources.
    LIB_OBJS
    BUILTIN_OBJS
    XDIFF_OBJS
    VCSSVN_OBJS
    TEST_BUILTINS_OBJS
    SCRIPT_SH
    SCRIPT_PERL
  3. Philip suggested to change the error message if sh/bash was not
    found on windows.
  4. CMake now tests for ICONV_OMITS_BOM, NO_ST_BLOCKS_IN_STRUCT_STAT
  5. Renamed the variable test_helper_sources to test-tool_SOURCES [PATCH 4/8]
    to be consistent with the naming of source variables.

Changes v2:
Changes 1,2,4 have been rebased to PATCH 01/xx
CMake uses GIT-VERSION-GEN to get the version of Git
Fixed a bug where a Windows user can pose as Linux user and vice versa. [PATCH 6/8]

Changes v3:
Patch changes are moved from the commit messages and are placed here.
Code inside check_c_source_(compiles/runs) have been formatted according to git coding guidelines. [PATCH 1/8]
The CMake script parses the Makefile for SCRIPT_LIB also. [PATCH 2/8]
The CMake script globs templates, po files. Logic has been added to place the template files in their respective directories instead of hard-coding them. [PATCH 2/8]

Changes v4:
Removed EXE_EXTENSION conditional stuff using CMAKE_EXECUTABLE_SUFFIX [PATCH 4/8]
There was an issue in build pipelines where CMake was not able to find the correct iconv library (caused by an update that installed PostgreSQL), so we need to manually set the location of the iconv library and its includes. This issue is extremely rare and is specific to the implementation of FindIconv.cmake. Other libraries are unaffected. [PATCH 8/8]

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

Welcome to GitGitGadget

Hi @SibiSiddharthan, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that your Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the FreeNode IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Freenode. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 23609c211489ef75aa04c9bde2a6772011dccd10:
The first line must be separated from the rest by an empty line

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit aa14a2b809cf5711b42379657dc1557f33844f17:
The first line must be separated from the rest by an empty line

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 5cacfb35092db009cd15f7f47f3e6846f24bb220:
The first line must be separated from the rest by an empty line

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 33a7d8cc0d911118d5d4ab6546fa5d3a5467c37d:
The first line must be separated from the rest by an empty line

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 0a33e04c69829a8e404b07a7118d7ea80b900871:
The first line must be separated from the rest by an empty line

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 5a38586821d585d07de4208a56ea164a1644ea91:
Prefixed commit message must be in lower case: CMake: Update

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 93fc2748ec9b002a671dedd51c62084041f1f9f2:
Prefixed commit message must be in lower case: CMake: Update

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 6bc50d7814ba3c388c6e4827bb6c10e5e58f5434:
Prefixed commit message must be in lower case: CMake: Support for testing with CTest

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 69b3c7eab57254835e3b9d1d2b21f4cea271a3bf:
Prefixed commit message must be in lower case: CMake: Support for testing when building out of the source tree.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 3474a22b7b45a001caa0f50baebfe4113c6b0ea9:
Prefixed commit message must be in lower case: CMake: Support for building on Windows with CMake, only MinGW support.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 4bc9fdc7864e31eb3b768c7479410dda42138be8:
Prefixed commit message must be in lower case: CMake: Added support for compiling Git on Windows with MSVC and Clang.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 65b8116669570ee93387668bc8c43b8bd784dc47:
Prefixed commit message must be in lower case: cmake: Support for testing with CTest

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit f920f3122316aeda5b3cf275f50d58bd75a50628:
Prefixed commit message must be in lower case: cmake: Support for testing when building out of the source tree.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 8a6fef1603f06593e976b6375ccaf45deef596aa:
Prefixed commit message must be in lower case: cmake: Support for building on Windows with CMake, only MinGW support.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 69e91a3af6c342e5bbb52147100aa24e4ce0ef49:
Prefixed commit message must be in lower case: cmake: Added support for compiling Git on Windows with MSVC and Clang.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

There is an issue in commit 6a33a1c6307109042d00f4360cdb8e89edd984ed:
Prefixed commit message must be in lower case: github: Modified main.yml to use CMake for Visual Studio build

@dscho
Copy link
Member

dscho commented Apr 22, 2020

Would you mind aligning the style of the commit messages more with the existing ones? For example,

CMake build system for git on Linux

should probably read more like

Introduce support for configuring on Linux via CMake

And instead of

CMake 3.14 is chosen because it offers a cross platform way of creating
hardlinks.

the commit message should be a lot more verbose. Personally, I would do something like this:

Introduce CMake support for configuring Git on Linux

At the moment, the recommended way to configure Git's builds is to
simply run `make`. If that does not work, the recommended strategy is to
look at the top of the `Makefile` to see whether any "Makefile knob" has
to be turned on/off, e.g. `make NO_OPENSSL=YesPlease`.

Alternatively, Git also has an `autoconf` setup which allows configuring
builds via `./configure [<option>...]`.

Both of these options are fine if the developer works on Unix or Linux.
But on Windows, we have to jump through hoops to configure a build
(read: we force the user to install a full Git for Windows SDK, which
occupies around two gigabytes (!) on disk and downloads about three
quarters of a gigabyte worth of Git objects).

To make this a little less awkward, the Git for Windows project offers
the `vs/master` branch which has a full Visual Studio solution generated
and committed. This branch can therefore be used to tinker with Git in
Visual Studio _without_ having to download the full Git for Windows SDK.
Unfortunatly, that branch is auto-generated from Git for Windows'
`master`. If a developer wants to tinker, say, with `pu`, they are out
of luck.

CMake was invented to make this sort of problem go away, by providing a
more standardized, cross-platform way to configure builds.

With a working support CMake, developers on Windows need only install
CMake, configure their build, load the generated Visual Studio solution
and immediately start modifying the code and build their own version of
Git. Likewise, developers on other platforms can use the convenient GUI
tools provided by CMake to configure their build.

So let's start building CMake support for Git.

This is only the first step, and to make it easier to review, it only
allows for configuring builds on the platform that is easiest to
configure for: Linux.

Note: earlier endeavors on the Git mailing list to introduce CMake ended
up in dead water. The primary reason for that was that reviewers
_expected_ CMake support to fall out of maintenance, unless the
contributor would promise to keep an eye on keeping CMake support up to
date. However, in the meantime, support for automated testing has been
introduced in Git's source code, and a later patch will modify the
(still experimental) GitHub workflow to continually verify that CMake
support is still complete. That will make maintenance reasonably easy.

Note: this patch asks for the minimum version v3.14 of CMake (which is
not all that old as of time of writing) because that is the first
version to offer a platform-independent way to generate hardlinks as
part of the build. This is needed to generate all those hardlinks for
the built-in commands of Git.

Ideally, all of these commit messages will be equally verbose and convincing.

(And their "onelines", i.e. the first lines of their commit messages, won't read "cmake: update" for the most part 😄 )

@dscho
Copy link
Member

dscho commented Apr 22, 2020

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

User SibiSiddharthan is now allowed to use GitGitGadget.

WARNING: SibiSiddharthan has no public email address set on GitHub

@dscho
Copy link
Member

dscho commented Apr 22, 2020

To add to my previous comment: The commit message

 cmake: update

Added generating shell, perl and python scripts with CMake

would probably read a lot better like this:

cmake: generate the shell/Perl/Python scripts

This patch implements the placeholder substitution to generate, say,
`git-pull-request` from `git-pull-request.sh`.

Note that we reimplement in a CMake-native way the `sed` invocation that
is defined as `cmd_munge_script` in the `Makefile`.

Following this example, the remainder of the commit messages are probably eager to be enhanced in the same manner 😄

@SibiSiddharthan
Copy link
Author

SibiSiddharthan commented Apr 22, 2020

Will reword the commit messages.

And I don't think the configure.ac script works anymore. I tried running it it does nothing(does not generate a makefile for out of source builds).

@SibiSiddharthan
Copy link
Author

SibiSiddharthan commented Apr 22, 2020

Have reworded the commit messages, explained what each patch does.
What else needs to be done?

@SibiSiddharthan
Copy link
Author

/preview

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 22, 2020

Preview email sent as pull.614.git.1587572038.gitgitgadget@gmail.com

@SibiSiddharthan
Copy link
Author

@dscho shall I submit for a PR?

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 24, 2020

This patch series was integrated into seen via git@fa27177.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 24, 2020

This patch series was integrated into seen via git@5700fce.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2020

This patch series was integrated into seen via git@da8ccd0.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2020

This patch series was integrated into seen via git@32df5b4.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 30, 2020

This patch series was integrated into seen via git@a2c02e8.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 30, 2020

This patch series was integrated into seen via git@4c893e5.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 30, 2020

This patch series was integrated into seen via git@445b247.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 31, 2020

This patch series was integrated into seen via git@1ecde87.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 1, 2020

This patch series was integrated into seen via git@10eb7db.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 1, 2020

This patch series was integrated into seen via git@c72951a.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 1, 2020

This patch series was integrated into next via git@ffaf583.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 3, 2020

This patch series was integrated into seen via git@553f48b.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2020

This patch series was integrated into next via git@a0d7016.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2020

This patch series was integrated into seen via git@bcefc73.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 5, 2020

This patch series was integrated into seen via git@4679dad.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 5, 2020

This patch series was integrated into seen via git@fb20794.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 7, 2020

This patch series was integrated into seen via git@6e512ef.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 10, 2020

This patch series was integrated into seen via git@ce9d76c.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 12, 2020

This patch series was integrated into seen via git@a30e4c5.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 12, 2020

This patch series was integrated into next via git@a30e4c5.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 12, 2020

This patch series was integrated into master via git@a30e4c5.

@gitgitgadget gitgitgadget bot added the master label Aug 12, 2020
@gitgitgadget gitgitgadget bot closed this Aug 12, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Aug 12, 2020

Closed via a30e4c5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants