[PATCH] cmake: ignore generated files

[PATCH] cmake: ignore generated files

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When using CMake to generate the files required to build Git in Visual
Studio, a bunch of files are generated. We will want to prevent them
from being tracked in Git.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    Ignore files generated by CMake

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-735%2Fdscho%2Fignore-generated-cmake-files-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-735/dscho/ignore-generated-cmake-files-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/735

 .gitignore | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.gitignore b/.gitignore
index 9673e792db..36f5ac4138 100644
--- a/.gitignore
+++ b/.gitignore
@@ -239,3 +239,9 @@ Release/
 /git.VC.VC.opendb
 /git.VC.db
 *.dSYM
+*.vcxproj
+*.vcxproj.filters
+/CMakeCache.txt
+/CMakeFiles/
+/*.cmake
+/DartConfiguration.tcl

base-commit: 54e85e7af1ac9e9a92888060d6811ae767fea1bc
-- 
gitgitgadget

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When using CMake to generate the files required to build Git in Visual
> Studio, a bunch of files are generated. We will want to prevent them
> from being tracked in Git.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     Ignore files generated by CMake
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-735%2Fdscho%2Fignore-generated-cmake-files-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-735/dscho/ignore-generated-cmake-files-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/735
>
>  .gitignore | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/.gitignore b/.gitignore
> index 9673e792db..36f5ac4138 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -239,3 +239,9 @@ Release/
>  /git.VC.VC.opendb
>  /git.VC.db
>  *.dSYM
> +*.vcxproj
> +*.vcxproj.filters
> +/CMakeCache.txt
> +/CMakeFiles/
> +/*.cmake
> +/DartConfiguration.tcl

Good to catch these cruft.  

Does the equivalent of "make distclean" need to be updated to clean
them as well, or is it sufficient to ignore the build procedure and
just rely on "git clean -f -x"?

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

Hi Junio,

On Thu, 17 Sep 2020, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When using CMake to generate the files required to build Git in Visual
> > Studio, a bunch of files are generated. We will want to prevent them
> > from being tracked in Git.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >     Ignore files generated by CMake
> >
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-735%2Fdscho%2Fignore-generated-cmake-files-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-735/dscho/ignore-generated-cmake-files-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/735
> >
> >  .gitignore | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/.gitignore b/.gitignore
> > index 9673e792db..36f5ac4138 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -239,3 +239,9 @@ Release/
> >  /git.VC.VC.opendb
> >  /git.VC.db
> >  *.dSYM
> > +*.vcxproj
> > +*.vcxproj.filters
> > +/CMakeCache.txt
> > +/CMakeFiles/
> > +/*.cmake
> > +/DartConfiguration.tcl
>
> Good to catch these cruft.
>
> Does the equivalent of "make distclean" need to be updated to clean
> them as well, or is it sufficient to ignore the build procedure and
> just rely on "git clean -f -x"?

Since CMake in conjunction with Visual Studio completely side-steps
`make`, I think it would make most sense to ignore `make distclean` in
this context and go for `git clean -dfx` instead.

Ciao,
Dscho

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Good to catch these cruft.
>>
>> Does the equivalent of "make distclean" need to be updated to clean
>> them as well, or is it sufficient to ignore the build procedure and
>> just rely on "git clean -f -x"?
>
> Since CMake in conjunction with Visual Studio completely side-steps
> `make`, I think it would make most sense to ignore `make distclean` in
> this context and go for `git clean -dfx` instead.

I think you misunderstood the question, overlooking the "equivalent"
part.

I expected that when CMake & VS discards build artifacts, it would
not make literal use of "make distclean".  After all, it does not
use "make all" to build, either.

That led me to suspect that CMake & VS may have a build target that
is used to discard build artifacts, the moral equivalent to "make
distclean".  That is where my question "if we are making .gitignore
aware of more crufts, don't we need to tell the machinery, which is
equivalent to 'make disclean', came from.

What I am hearing here is that people with CMake & VS use "git clean
-dfx" when they want to go back to the pristine state, unlike those
who use "make distclean", and there is nothing to adjust for newly
discovered crufts we are leaving on the filesystem.  

If that is the case, it is 100% fine.  It was that I just didn't
expect not having a "remove cruft" rule in the build procedure.

Thanks.

Re: [PATCH] cmake: ignore generated files

[Đoàn Trần Công Danh]

On 2020-09-18 08:28:48-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Good to catch these cruft.
> >>
> >> Does the equivalent of "make distclean" need to be updated to clean
> >> them as well, or is it sufficient to ignore the build procedure and
> >> just rely on "git clean -f -x"?
> >
> > Since CMake in conjunction with Visual Studio completely side-steps
> > `make`, I think it would make most sense to ignore `make distclean` in
> > this context and go for `git clean -dfx` instead.
> 
> I think you misunderstood the question, overlooking the "equivalent"
> part.
> 
> I expected that when CMake & VS discards build artifacts, it would
> not make literal use of "make distclean".  After all, it does not
> use "make all" to build, either.

Actually, in CMake land, people usually do this instead:

	mkdir build
	cmake [-Ggenerator] ..
	<make/ninja/msbuild>

Then, when they want to run something equivalent with make distclean,
they run:

	cd ..
	rm -rf build

instead.

The change that Dscho suggested was meant for those people that run
CMake in same directory of source dir, which is mostly discouraged
in CMake land.

In additional, CMake's default Generator in *nix is Unix Makefiles,
which will clash with our Makefile, and totally unsupported.

I think the original CMake proposal didn't touch .gitignore because
they run in a separated build-dir.

-- 
Danh

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> Actually, in CMake land, people usually do this instead:
>
> 	mkdir build
> 	cmake [-Ggenerator] ..
> 	<make/ninja/msbuild>

I presume you "cd build" between steps 1 & 2?  I take the "people
usually do" is the best current practice?

> Then, when they want to run something equivalent with make distclean,
> they run:
>
> 	cd ..
> 	rm -rf build
>
> instead.
>
> The change that Dscho suggested was meant for those people that run
> CMake in same directory of source dir, which is mostly discouraged
> in CMake land.
>
> In additional, CMake's default Generator in *nix is Unix Makefiles,
> which will clash with our Makefile, and totally unsupported.

I recall that our CMakeLists file asks the top-level Makefile about
basic things so that we do not have to duplicate "list of files"
etc.  in places, so I can understand that it would lead to a
disaster to do "cmake -Ggenerator" at the top-level.

> I think the original CMake proposal didn't touch .gitignore because
> they run in a separated build-dir.

If so, not only my "do we need a matching change to CMakeLists to
teach how to clean crufts?" becomes unnecessary, but the original
patch that started this thread to touch .gitignore at the top level,
does, too.

I wonder what led Dscho not to follow the "create a 'build' dir and
do things there" practice.  Judging from the fact that the "because
they run in a separate build directory" assumption did not hold to
somebody as experienced as Dscho, it is likely others would do the
same.

What can we do to make it easier for people to discover and follow
the best current practice?  Are there things in our build
instruction document (e.g. the comment at the top of CMakeLists.txt)
that needs updating?

Thanks.

Re: [PATCH] cmake: ignore generated files

[Đoàn Trần Công Danh]

On 2020-09-18 09:21:45-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
> 
> > Actually, in CMake land, people usually do this instead:
> >
> > 	mkdir build
> > 	cmake [-Ggenerator] ..
> > 	<make/ninja/msbuild>
> 
> I presume you "cd build" between steps 1 & 2?

Opps, yes, there is a "cd build" there, and it should be:

	cmake [-G<generator>] ..

<generator> is the target build system, default to "Unix Makefiles"
in *nix land.

> I take the "people usually do" is the best current practice?

I would say it's the best practice. It's recommended in cmake official
guide (although, they don't write the word "best practice") [1]

	It is recommended to build in a separate directory to the
	source because that keeps the source directory pristine,
	allows for building a single source with multiple toolchains,
	and allows easy clearing of build artifacts by simply deleting
	the build directory.

> 
> > Then, when they want to run something equivalent with make distclean,
> > they run:
> >
> > 	cd ..
> > 	rm -rf build
> >
> > instead.
> >
> > The change that Dscho suggested was meant for those people that run
> > CMake in same directory of source dir, which is mostly discouraged
> > in CMake land.
> >
> > In additional, CMake's default Generator in *nix is Unix Makefiles,
> > which will clash with our Makefile, and totally unsupported.
> 
> I recall that our CMakeLists file asks the top-level Makefile about
> basic things so that we do not have to duplicate "list of files"
> etc.  in places, so I can understand that it would lead to a
> disaster to do "cmake -Ggenerator" at the top-level.

Yes, and in some first iteration, the proposal didn't check for
builddir != sourcedir when -G"Unix Makefiles", which was corrected later.

I haven't checked, but I could imagine that "cmake -Gninja" at top-level
sourcedir should work without problems, though.

> > I think the original CMake proposal didn't touch .gitignore because
> > they run in a separated build-dir.
> 
> If so, not only my "do we need a matching change to CMakeLists to
> teach how to clean crufts?" becomes unnecessary, but the original
> patch that started this thread to touch .gitignore at the top level,
> does, too.
> 
> I wonder what led Dscho not to follow the "create a 'build' dir and
> do things there" practice.  Judging from the fact that the "because
> they run in a separate build directory" assumption did not hold to
> somebody as experienced as Dscho, it is likely others would do the
> same.
> 
> What can we do to make it easier for people to discover and follow
> the best current practice?  Are there things in our build
> instruction document (e.g. the comment at the top of CMakeLists.txt)
> that needs updating?

I think Sibi had done a lot of good work to write instruction at the
top of CMakeLists.txt, I guess it's a bit too much instruction.
The instruction for out of source build was written after the
instruction to build in-tree.

Should we change the order?  I don't really know.

I only tried to build Git with CMake only one during the review process.
And haven't touched it ever since.

1: https://cmake.org/cmake/help/v3.18/guide/user-interaction/index.html#command-line-cmake-tool

-- 
Danh

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>> What can we do to make it easier for people to discover and follow
>> the best current practice?  Are there things in our build
>> instruction document (e.g. the comment at the top of CMakeLists.txt)
>> that needs updating?
>
> I think Sibi had done a lot of good work to write instruction at the
> top of CMakeLists.txt, I guess it's a bit too much instruction.
> The instruction for out of source build was written after the
> instruction to build in-tree.
>
> Should we change the order?  I don't really know.

Oh, absolutely.  

It begins with

    cmake `relative-path-to-CMakeLists.txt` -DCMAKE_BUILD_TYPE=Release
    Eg.
    From the root of git source tree
            `cmake contrib/buildsystems/ `
    This will build the git binaries at the root

which tells exactly what Dscho did and got him writing the patch in
question.  You'd need to keep reading to be merely _aware_ that it
is possible to do an out-of-tree build, without getting even told
that it is the norm in CMake land.

We should just remove the above and start the comment seciton with
something more like this:

    When using CMake, building in place is highly discouraged.
    Instead, create a new "git-build" directory and perform your
    build there, like this (from the top of the working tree):

            $ mkdir git-build
            $ cd git-build
            $ cmake ../contrib/buildsystems/ -DCMAKE_BUILD_TYPER=Release

    This will build the git binaries in git-build directory

Doing so would reduce the number of choices the user has to make,
and reduce the confusion.

Thanks.

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

Hi Junio,

On Fri, 18 Sep 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> Good to catch these cruft.
> >>
> >> Does the equivalent of "make distclean" need to be updated to clean
> >> them as well, or is it sufficient to ignore the build procedure and
> >> just rely on "git clean -f -x"?
> >
> > Since CMake in conjunction with Visual Studio completely side-steps
> > `make`, I think it would make most sense to ignore `make distclean` in
> > this context and go for `git clean -dfx` instead.
>
> I think you misunderstood the question, overlooking the "equivalent"
> part.
>
> I expected that when CMake & VS discards build artifacts, it would
> not make literal use of "make distclean".  After all, it does not
> use "make all" to build, either.
>
> That led me to suspect that CMake & VS may have a build target that
> is used to discard build artifacts, the moral equivalent to "make
> distclean".  That is where my question "if we are making .gitignore
> aware of more crufts, don't we need to tell the machinery, which is
> equivalent to 'make disclean', came from.
>
> What I am hearing here is that people with CMake & VS use "git clean
> -dfx" when they want to go back to the pristine state, unlike those
> who use "make distclean", and there is nothing to adjust for newly
> discovered crufts we are leaving on the filesystem.

Yes, that is my understanding.

> If that is the case, it is 100% fine.  It was that I just didn't
> expect not having a "remove cruft" rule in the build procedure.

Thanks,
Dscho

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 3485 bytes --]

Hi Junio,

On Fri, 18 Sep 2020, Junio C Hamano wrote:

> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
>
> > Actually, in CMake land, people usually do this instead:
> >
> > 	mkdir build
> > 	cmake [-Ggenerator] ..
> > 	<make/ninja/msbuild>
>
> I presume you "cd build" between steps 1 & 2?  I take the "people
> usually do" is the best current practice?
>
> > Then, when they want to run something equivalent with make distclean,
> > they run:
> >
> > 	cd ..
> > 	rm -rf build
> >
> > instead.
> >
> > The change that Dscho suggested was meant for those people that run
> > CMake in same directory of source dir, which is mostly discouraged
> > in CMake land.

It is discouraged, but not disallowed.

> > In additional, CMake's default Generator in *nix is Unix Makefiles,
> > which will clash with our Makefile, and totally unsupported.
>
> I recall that our CMakeLists file asks the top-level Makefile about
> basic things so that we do not have to duplicate "list of files"
> etc.  in places, so I can understand that it would lead to a
> disaster to do "cmake -Ggenerator" at the top-level.

Indeed, when asking CMake to generate a Makefile twice, there will be a
problem.

But so far, we successfully resisted making the CMake support powerful
enough to support anything but Windows and Visual C.

> > I think the original CMake proposal didn't touch .gitignore because
> > they run in a separated build-dir.
>
> If so, not only my "do we need a matching change to CMakeLists to
> teach how to clean crufts?" becomes unnecessary, but the original
> patch that started this thread to touch .gitignore at the top level,
> does, too.
>
> I wonder what led Dscho not to follow the "create a 'build' dir and
> do things there" practice.  Judging from the fact that the "because
> they run in a separate build directory" assumption did not hold to
> somebody as experienced as Dscho, it is likely others would do the
> same.

That's because Dscho does not like the separate build directory, even if
they know that in the CMake world, it is kind of expected.

It's just that it would be super convenient for Visual Studio users to
just generate their project files in-place. That's why I started down that
road.

> What can we do to make it easier for people to discover and follow
> the best current practice?  Are there things in our build
> instruction document (e.g. the comment at the top of CMakeLists.txt)
> that needs updating?

Ideally, we would tell Visual Studio users to "just install CMake, start
its GUI, direct it to the Git source, configure and generate". Alas, it is
not that easy:

- The `SH_EXE` is not found by default (`C:\Program Files\Git\bin\sh.exe`
  should be used in the vast majority of the cases),
- If the build directory is left unspecified, the non-writable `C:\Program
  Files\CMake\bin` directory is used,
- The `compat\vcbuild\vcpkg` system is not initialized automatically, and
  even if the user initialized it, the dependencies (such as expat, zlib)
  are still not found.

I would like to make things easier, and forcing users to use a separate
build directory (that needs to be outside of the Git source tree because
our `.gitignore` does not handle it well) would go the other direction, I
fear.

In short: this `.gitignore` is but one small step in the endeavor to make
it more convenient for Visual Studio users to contribute.

Ciao,
Dscho

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> What I am hearing here is that people with CMake & VS use "git clean
>> -dfx" when they want to go back to the pristine state, unlike those
>> who use "make distclean", and there is nothing to adjust for newly
>> discovered crufts we are leaving on the filesystem.
>
> Yes, that is my understanding.

I do not know if you have read other messages in the thread, but my
current understanding of what we learned in this thread [*1*] is
that those who use CMake, especially in a tree like ours that has
its own Makefiles, (sh|w)ould create a throw-away directory and run
build there, so even "git clean -dxf" is not part of their "clean"
procedure.


[Footnote]

*1* https://lore.kernel.org/git/20200919004030.GB1837@danh.dev/

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

Hi Junio,

On Mon, 21 Sep 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> What I am hearing here is that people with CMake & VS use "git clean
> >> -dfx" when they want to go back to the pristine state, unlike those
> >> who use "make distclean", and there is nothing to adjust for newly
> >> discovered crufts we are leaving on the filesystem.
> >
> > Yes, that is my understanding.
>
> I do not know if you have read other messages in the thread,

I did.

> but my current understanding of what we learned in this thread [*1*] is
> that those who use CMake, especially in a tree like ours that has its
> own Makefiles, (sh|w)ould create a throw-away directory and run build
> there, so even "git clean -dxf" is not part of their "clean" procedure.

Well, our CMake support does not extend to generating Makefiles: we
specifically target only Windows withour CMake support, and since there is
no need to help developers who already use the Git for Windows SDK or
Cygwin, we really focus only on supporting Visual Studio.

In that respect, I think that all that talk about Makefiles isn't quite
spot on.

In any case, `git clean -dfx` strikes me as a perfectly acceptable
equivalent of `make clean` when no `make` is available, as is the case in
Visual Studio's case.

Ciao,
Dscho

> [Footnote]
>
> *1* https://lore.kernel.org/git/20200919004030.GB1837@danh.dev/
>

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > The change that Dscho suggested was meant for those people that run
>> > CMake in same directory of source dir, which is mostly discouraged
>> > in CMake land.
>
> It is discouraged, but not disallowed.
> ...
>> > I think the original CMake proposal didn't touch .gitignore because
>> > they run in a separated build-dir.
>>
>> If so, not only my "do we need a matching change to CMakeLists to
>> teach how to clean crufts?" becomes unnecessary, but the original
>> patch that started this thread to touch .gitignore at the top level,
>> does, too.
>>
>> I wonder what led Dscho not to follow the "create a 'build' dir and
>> do things there" practice.  Judging from the fact that the "because
>> they run in a separate build directory" assumption did not hold to
>> somebody as experienced as Dscho, it is likely others would do the
>> same.
>
> That's because Dscho does not like the separate build directory, even if
> they know that in the CMake world, it is kind of expected.

Sorry, but that does not sound like a convincing excuse because ...

> It's just that it would be super convenient for Visual Studio users to
> just generate their project files in-place. That's why I started down that
> road.
> ...
> Ideally, we would tell Visual Studio users to "just install CMake, start
> its GUI, direct it to the Git source, configure and generate". Alas, it is
> not that easy:
>
> - The `SH_EXE` is not found by default (`C:\Program Files\Git\bin\sh.exe`
>   should be used in the vast majority of the cases),
> - If the build directory is left unspecified, the non-writable `C:\Program
>   Files\CMake\bin` directory is used,
> - The `compat\vcbuild\vcpkg` system is not initialized automatically, and
>   even if the user initialized it, the dependencies (such as expat, zlib)
>   are still not found.

... if the build directory needs to be specified anyway, there don't
seem to be a big difference between telling them to create an empty
build place and use it and telling them to point at the source tree
itself, so ...

> I would like to make things easier, and forcing users to use a separate
> build directory (that needs to be outside of the Git source tree because
> our `.gitignore` does not handle it well) would go the other direction, I
> fear.

... the above sounds like the argument concentrates too much on
where the build directory is (i.e. between "in place" and "a
throw-away directory next door"), which sounds like much smaller
point compared to the other things that needs to be improved in the
VS users.  And making a choice against what is recommended as best
practice...?  I dunno.

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When using CMake to generate the files required to build Git in Visual
> Studio, a bunch of files are generated. We will want to prevent them
> from being tracked in Git.

If we were to go this route, let's

 * update the log message to say that we assume we will use CMake to
   build in-place.

 * update the comment in the top part of the CMakeList.txt that
   suggests it is a possibility to use it from subdirectories.

 * optionally, update the same comment to recommend "clean -xdf"
   as a way to bring the source tree back to the pristine state.

Will queue with a tentative attempt to do the first bullet point to
save one roundtip, something like:

    When using CMake to generate the files required to build Git
    in-place for Visual Studio, ...

Other two points needs help from others (iow, I won't do so myself
right now).

Thanks.

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> When using CMake to generate the files required to build Git in Visual
>> Studio, a bunch of files are generated. We will want to prevent them
>> from being tracked in Git.
>
> If we were to go this route, let's
>
>  * update the log message to say that we assume we will use CMake to
>    build in-place.
>
>  * update the comment in the top part of the CMakeList.txt that
>    suggests it is a possibility to use it from subdirectories.

Sorry, sent too soon.  Either s/update/remove/, or to say that
this project assume things are built in-place.

>  * optionally, update the same comment to recommend "clean -xdf"
>    as a way to bring the source tree back to the pristine state.
>
> Will queue with a tentative attempt to do the first bullet point to
> save one roundtip, something like:
>
>     When using CMake to generate the files required to build Git
>     in-place for Visual Studio, ...
>
> Other two points needs help from others (iow, I won't do so myself
> right now).
>
> Thanks.

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

Hi Junio,

On Wed, 23 Sep 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > The change that Dscho suggested was meant for those people that run
> >> > CMake in same directory of source dir, which is mostly discouraged
> >> > in CMake land.
> >
> > It is discouraged, but not disallowed.
> > ...
> >> > I think the original CMake proposal didn't touch .gitignore because
> >> > they run in a separated build-dir.
> >>
> >> If so, not only my "do we need a matching change to CMakeLists to
> >> teach how to clean crufts?" becomes unnecessary, but the original
> >> patch that started this thread to touch .gitignore at the top level,
> >> does, too.
> >>
> >> I wonder what led Dscho not to follow the "create a 'build' dir and
> >> do things there" practice.  Judging from the fact that the "because
> >> they run in a separate build directory" assumption did not hold to
> >> somebody as experienced as Dscho, it is likely others would do the
> >> same.
> >
> > That's because Dscho does not like the separate build directory, even if
> > they know that in the CMake world, it is kind of expected.
>
> Sorry, but that does not sound like a convincing excuse because ...
>
> > It's just that it would be super convenient for Visual Studio users to
> > just generate their project files in-place. That's why I started down that
> > road.
> > ...
> > Ideally, we would tell Visual Studio users to "just install CMake, start
> > its GUI, direct it to the Git source, configure and generate". Alas, it is
> > not that easy:
> >
> > - The `SH_EXE` is not found by default (`C:\Program Files\Git\bin\sh.exe`
> >   should be used in the vast majority of the cases),
> > - If the build directory is left unspecified, the non-writable `C:\Program
> >   Files\CMake\bin` directory is used,
> > - The `compat\vcbuild\vcpkg` system is not initialized automatically, and
> >   even if the user initialized it, the dependencies (such as expat, zlib)
> >   are still not found.
>
> ... if the build directory needs to be specified anyway, there don't

It doesn't need to be specified.

> seem to be a big difference between telling them to create an empty
> build place and use it and telling them to point at the source tree
> itself, so ...
>
> > I would like to make things easier, and forcing users to use a separate
> > build directory (that needs to be outside of the Git source tree because
> > our `.gitignore` does not handle it well) would go the other direction, I
> > fear.
>
> ... the above sounds like the argument concentrates too much on
> where the build directory is (i.e. between "in place" and "a
> throw-away directory next door"), which sounds like much smaller
> point compared to the other things that needs to be improved in the
> VS users.  And making a choice against what is recommended as best
> practice...?  I dunno.

All I want is for the CMake support to be easier to use, yet we go in the
opposite direction: instead of allowing to use CMake under more
circumstances (which actually *works*, we just don't have the appropriate
patterns in our `.gitignore` yet to avoid adding and committing the
generated files), we now seem to intend to require a separate build
directory.

That's the opposite direction of making things more convenient for Visual
Studio users.

Ciao,
Dscho

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> All I want is for the CMake support to be easier to use, yet we go in the
> opposite direction: instead of allowing to use CMake under more
> circumstances (which actually *works*, we just don't have the appropriate

Not, "more", but what you are doing is to ensure it is used at only
one single place, which is the top-level (and nowhere else, judging
from the .gitignore additions).

And that is fine---if you were to add .gitignore entries, you cannot
leave it up to the end-users where the build happens.
  
So, let's not pretend that you are allowing "more circumstances".
Forcing a single choice to make things predictable is fine, but
let's explain it as such, so that people won't be confused into
thinking that they can follow their experiences gained from using
CMake in other projects that lets them build in a separate
directory.  The other things in our project, including the
patterns added to .gitignore with the patch in question, are not set
up to allow that.

Re: [PATCH] cmake: ignore generated files

[SZEDER Gábor]

On Wed, Sep 23, 2020 at 03:08:26PM +0200, Johannes Schindelin wrote:
> In any case, `git clean -dfx` strikes me as a perfectly acceptable
> equivalent of `make clean` when no `make` is available, as is the case in
> Visual Studio's case.

Not at all: 'make (dist)clean' only removes build artifacts and leaves
untracked files intact, while 'git clean -dfx' removes untracked files
as well, so using the latter instead of the former might easily lead to
data loss.

Re: [PATCH] cmake: ignore generated files

[Đoàn Trần Công Danh]

Hi Dscho,

On 2020-09-23 22:27:17+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > ... the above sounds like the argument concentrates too much on
> > where the build directory is (i.e. between "in place" and "a
> > throw-away directory next door"), which sounds like much smaller
> > point compared to the other things that needs to be improved in the
> > VS users.  And making a choice against what is recommended as best
> > practice...?  I dunno.
> 
> All I want is for the CMake support to be easier to use, yet we go in the
> opposite direction: instead of allowing to use CMake under more
> circumstances (which actually *works*, we just don't have the appropriate
> patterns in our `.gitignore` yet to avoid adding and committing the
> generated files), we now seem to intend to require a separate build
> directory.

I've left Windows development land for a long time.
So, please take below discussion with grain of salt.

When I was there, CMake Users on Windows mostly used CMake-GUI to
generate build system for CMake since running CMake as CLI in Windows
takes too much hassle.

When I was there, CMake-GUI shows the option to choose build directories
explicitly, and whenever the source directories changed, the build
directories also changed, with some [-/]build added into sourcedir [1]

I heard that nowaday, CMake is supported natively with MSVC, I don't
know what is the default option when using CMake with MSVC, but from
the history of MSVC always supports building out of tree, and
information for Microsoft Docs [2]:

	Click the Show All Files button at the top of Solution
	Explorer to see all the CMake-generated output in the
	out/build/<config> folders.

I think the default UX with CMake on Windows is building project out
of tree.

> That's the opposite direction of making things more convenient for Visual
> Studio users.

So, I don't think we would provide them more convenient with this change.


1: https://cmake.org/runningcmake/
2: https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

-- 
Danh

Re: [PATCH] cmake: ignore generated files

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Wed, Sep 23, 2020 at 03:08:26PM +0200, Johannes Schindelin wrote:
>> In any case, `git clean -dfx` strikes me as a perfectly acceptable
>> equivalent of `make clean` when no `make` is available, as is the case in
>> Visual Studio's case.
>
> Not at all: 'make (dist)clean' only removes build artifacts and leaves
> untracked files intact, while 'git clean -dfx' removes untracked files
> as well, so using the latter instead of the former might easily lead to
> data loss.

Well, with the patch in question applied, "git clean -f" would be usable
as a poor-man's replacement that removes "ignored and expendable" paths
that match patterns listed in the file, so...

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 3877 bytes --]

Hi Danh,

On Thu, 24 Sep 2020, Đoàn Trần Công Danh wrote:

> On 2020-09-23 22:27:17+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > ... the above sounds like the argument concentrates too much on
> > > where the build directory is (i.e. between "in place" and "a
> > > throw-away directory next door"), which sounds like much smaller
> > > point compared to the other things that needs to be improved in the
> > > VS users.  And making a choice against what is recommended as best
> > > practice...?  I dunno.
> >
> > All I want is for the CMake support to be easier to use, yet we go in the
> > opposite direction: instead of allowing to use CMake under more
> > circumstances (which actually *works*, we just don't have the appropriate
> > patterns in our `.gitignore` yet to avoid adding and committing the
> > generated files), we now seem to intend to require a separate build
> > directory.
>
> I've left Windows development land for a long time.
> So, please take below discussion with grain of salt.
>
> When I was there, CMake Users on Windows mostly used CMake-GUI to
> generate build system for CMake since running CMake as CLI in Windows
> takes too much hassle.
>
> When I was there, CMake-GUI shows the option to choose build directories
> explicitly, and whenever the source directories changed, the build
> directories also changed, with some [-/]build added into sourcedir [1]

In my tests, the build directory was left empty. When I clicked the button
next to it, it defaulted to the same directory as `CMakeLists.txt`:
`contrib/buildsystems/`.

I might be holding this thing wrong, but if I don't, then we would
actually have to add a _different_ set of patterns to `.gitignore`.

> I heard that nowaday, CMake is supported natively with MSVC, I don't
> know what is the default option when using CMake with MSVC, but from
> the history of MSVC always supports building out of tree, and
> information for Microsoft Docs [2]:
>
> 	Click the Show All Files button at the top of Solution
> 	Explorer to see all the CMake-generated output in the
> 	out/build/<config> folders.

Oh wow, I missed this. And it looks promising: when I open a fresh
checkout of current git/git's `master` branch in a freshly updated Visual
Studio 2019, it finds the CMakeLists.txt file automatically.

But that's where the happy news end: it stops with the error message:

CMake Error at [...]\contrib\buildsystems\CMakeLists.txt:46 (message):
  sh: shell interpreter was not found in your path, please install one.On
  Windows, you can get it as part of 'Git for Windows' install at
  https://gitforwindows.org/	git
[...]\contrib\buildsystems\CMakeLists.txt	46

So there's a bit of work left to do for me.

> I think the default UX with CMake on Windows is building project out
> of tree.

Indeed, it _does_ create `contrib/buildsystems/out/` and starts outputting
files to `contrib/buildsystems/out/build/x64-Debug (default)/`.

Seeing as using Visual Studio's built-in CMake support is much more
convenient to use than a separate CMake installation, I reconsidered my
original idea, and now think that y'all are right, my current patch isn't
the best way forward. I'll rework the patch into a proper patch series
that takes into account what I learned today.

> > That's the opposite direction of making things more convenient for Visual
> > Studio users.
>
> So, I don't think we would provide them more convenient with this change.

Indeed. And more convenience is what I want, I don't want developers on
Windows to struggle with the tooling when all they want to do is to
contribute to Git.

Thank you,
Dscho

>
>
> 1: https://cmake.org/runningcmake/
> 2: https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019
>
> --
> Danh
>

Re: [PATCH] cmake: ignore generated files

[Johannes Schindelin]

Hi Junio,

On Wed, 23 Sep 2020, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > All I want is for the CMake support to be easier to use, yet we go in the
> > opposite direction: instead of allowing to use CMake under more
> > circumstances (which actually *works*, we just don't have the appropriate
>
> Not, "more", but what you are doing is to ensure it is used at only
> one single place, which is the top-level (and nowhere else, judging
> from the .gitignore additions).
>
> And that is fine---if you were to add .gitignore entries, you cannot
> leave it up to the end-users where the build happens.
>
> So, let's not pretend that you are allowing "more circumstances".
> Forcing a single choice to make things predictable is fine, but
> let's explain it as such, so that people won't be confused into
> thinking that they can follow their experiences gained from using
> CMake in other projects that lets them build in a separate
> directory.  The other things in our project, including the
> patterns added to .gitignore with the patch in question, are not set
> up to allow that.

While I respectfully disagree that I force a single choice on anybody, I
do agree that there is value in having one recommended route that is well
supported.

Thanks,
Dscho