Reinstate support for Visual Studio #287
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dscho
added
the
needs-work
|
Note to self: I still need to investigate whether that the "WIP" commit is still needed (I expect not). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
We ran out GUIDs. Again. But there is no need to: we can generate them semi-randomly from the target file name of the project. Note: the Vcproj generator is probably only interesting for historical reasons; nevertheless, the upcoming Vcxproj generator (to support modern Visual Studio versions) is based on the Vcproj generator and it is better to fix this here first. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Visual Studio takes the first listed application/library as the default startup project [1]. Detect the 'git' project and place it at the head of the project list, rather than at the tail. Export the apps list before libs list for both the projects and global structures of the .sln file. [1] http://stackoverflow.com/questions/1238553/ vs2008-where-is-the-startup-project-setting-stored-for-a-solution "In the solution file, there are a list of pseudo-XML "Project" entries. It turns out that whatever is the first one ends up as the Startup Project, unless it’s overridden in the suo file. Argh. I just rearranged the order in the file and it’s good." "just moving the pseudo-xml isn't enough. You also have to move the group of entries in the "GlobalSection(ProjectConfigurationPlatforms) = postSolution" group that has the GUID of the project you moved to the top. So there are two places to move lines." Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It is not necessary, and Visual Studio 2015 no longer supports it, anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Since 4b623d8 (MSVC: link in invalidcontinue.obj for better POSIX compatibility, 2014-03-29), invalidcontinue.obj is linked in the MSVC build, but it was not parsed correctly by the buildsystem. Ignore it, as it is known to Visual Studio and will be handled elsewhere. Also only substitute filenames ending with .o when generating the source .c filename, otherwise we would start to expect .cbj files to generate .obj files (which are not generated by our build)... In the future there may be source files that produce .obj files so keep the two issues (.obj files with & without source files) separate. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Duncan Smart <duncan.smart@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The Generators/ directory can contain spurious files such as editors' backup files. Even worse, there could be .swp files which are not even valid Perl scripts. Let's just ignore anything but .pm files in said directory. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The error message talked about a "lib option", but it clearly referred to a link option. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The engine.pl script expects file names not to contain spaces. However, paths with spaces are quite prevalent on Windows. Use shellwords() rather than split() to parse them correctly. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's build contains steps to handle internationalization. This caused hiccups in the parser used to generate QMake/Visual Studio project files. As those steps are irrelevant in this context, let's just ignore them. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Rather than swallowing the errors, it is better to have them in a file. To make it obvious what this is about, use the file name 'msvc-build-makedryerrors.txt'. Further, if the output is empty, simply delete that file. As we target Git for Windows' SDK (which, unlike its predecessor msysGit, offers Perl versions newer than 5.8), we can use the quite readable syntax `if -f -z $ErrsFile` (available in Perl >=5.10). Note that the file will contain the new values of the GIT_VERSION and GITGUI_VERSION if they were generated by the make file. They are omitted if the release is tagged and indentically defined in their respective GIT_VERSION_GEN file DEF_VER variables. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Add an option for capturing the output of the make dry-run used in determining the msvc-build structure for easy debugging. You can use the output of `--make-out <path>` in subsequent runs via the `--in <path>` option. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
One time too many did this developer call the `generate` script passing a `--make-out=<PATH>` option that was happily ignored (because there should be a space, not an equal sign, between `--make-out` and the path). And one time too many, this script not only ignored it but did not even complain. Let's fix that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Upon seeing the '-lcurl' option, point to the libcurl.lib. While there, fix the elsif indentation. Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git's test suite shows tons of breakages unless Git is compiled *without* NO_ICONV. That means, in turn, that we need to generate build definitions *with* libiconv, which in turn implies that we have to handle the -liconv option properly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is a dependency required for the non-smart HTTP backend. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
With the recent changes to allow building with MSVC=1, we now pass the /OPT:REF option to the compiler. This confuses the parser that wants to turn the output of a dry run into project definitions for QMake and Visual Studio: Unhandled link option @ line 213: /OPT:REF at [...] Let's just extend the code that passes through options that start with a dash, so that it passes through options that start with a slash, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
/submit |
|
Submitted as pull.287.git.gitgitgadget@gmail.com |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
t/t5505-remote.sh
Outdated
| @@ -824,6 +824,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' ' | |||
| ( | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> It is a real old anachronism from the Cogito days to have a
> .git/branches/ directory. And to have tests that ensure that Cogito
> users can migrate away from using that directory.
>
> But so be it, let's continue testing it.
>
> Let's make sure, however, that git init does not need to create that
> directory.
>
> This bug was noticed when testing with templates that had been
> pre-committed, skipping the empty branches/ directory of course because
> Git does not track empty directories.
Many tests assume that the .git/info/ directory exists, and the only
reason why they can is because we have templates/info--exclude; the
situation around .git/branches/ is exactly the same.
Deprecating .git/branches/ directory and dropping the parts of tests
that wants to make sure the support still works (t5516 is not about
migration but about working with .git/branches configuration) would
be a different matter. Until that happens, let's not do this patch;
otherwise it would force us to sprinkle "mkdir -p .git/info/" all
over the place for the same rationale. A directory in .git/ created
by hardcoded mkdir in "git init" (like .git/refs/) and created by
copying templates by "git init" (like .git/info/ and .git/branches)
are both "created by 'git init'". Special casing the latter is just
silly.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Thu, 18 Jul 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > It is a real old anachronism from the Cogito days to have a
> > .git/branches/ directory. And to have tests that ensure that Cogito
> > users can migrate away from using that directory.
> >
> > But so be it, let's continue testing it.
> >
> > Let's make sure, however, that git init does not need to create that
> > directory.
> >
> > This bug was noticed when testing with templates that had been
> > pre-committed, skipping the empty branches/ directory of course becaus=
e
> > Git does not track empty directories.
>
> Many tests assume that the .git/info/ directory exists, and the only
> reason why they can is because we have templates/info--exclude; the
> situation around .git/branches/ is exactly the same.
No, the situation with `info/` is that it includes a file called
`exclude`, which means that the `templates/` directory can be "tracked".
This is important because we want to commit all those generated files
that cannot be generated inside Visual Studio without Git for Windows'
SDK (which weighs in with several hundred megabytes to download, a
gigabyte on disk).
The `branches/` subdirectory, in contrast, is totally useless for at
least 99.999% of the users, and hence it is understandable that it does
not even contain a single file. Which means that it is *not* "tracked".
So when checking out, e.g. the `vs/master` branch at
https://github.com/git-for-windows/git, which is auto-generated using
this here patch series from Git for Windows' `master`, to allow building
in Visual Studio without having to download the full Git for Windows
SDK, there will be the `info/` directory (by virtue of containing a
trackable file) exists, and the `branches/` subdirectory won't exist.
As a consequence, the test scripts indicated in the commit message,
which _can_ be run in a regular Git for Windows (no Git for Windows SDK
required), will fail without this patch.
> Deprecating .git/branches/ directory and dropping the parts of tests
> that wants to make sure the support still works (t5516 is not about
> migration but about working with .git/branches configuration) would
> be a different matter.
Indeed.
You probably forgot that I already proposed that, long time ago:
https://public-inbox.org/git/cover.1494509599.git.johannes.schindelin@gmx.=
de/
I haven't forgotten, because you shot it down unceremoniously:
The last time I thought about trying this several years ago, I found
that people who need to grab things from many places still do use
.git/branches/ and their use case is hard to migrate to .git/config,
primarily because the former is "one per file" and it is easy to
add/remove/tweak without affecting others. Ask akpm@ if he still
prefers to use .git/branches/ for example.
Of course, with such a kind of argument, there is no way how I could
possibly prove that it is no longer advisable to have `.git/branches/`.
"Someone, somewhere _probably_ finds this still useful, so we won't
remove it."
> Until that happens, let's not do this patch;
Well, that's another of these type of arguments.
Without this patch, and without removing support for `.git/branches/`,
you force the Visual Studio build to _not_ pass the test suite. It's as
simple as that.
So basically, by this statement you decided that there will not be a
fully-functional way to build Git in Visual Studio and to pass the test
suite with that.
Which is a shame.
> otherwise it would force us to sprinkle "mkdir -p .git/info/" all
> over the place for the same rationale.
You may have assumed that I did not carefully verify that the test suite
passes with these patches. But I did. And I would not have submitted
this patch series if such a patch was necessary, at least not without
said patch.
> A directory in .git/ created by hardcoded mkdir in "git init" (like
> .git/refs/) and created by copying templates by "git init" (like
> .git/info/ and .git/branches) are both "created by 'git init'".
> Special casing the latter is just silly.
The thing that _really_ is silly is that we have that directory in
Git's templates, still.
Git itself does not populate it. Git does not need it, ever. Git works
Just Fine without it, and it is really by design that it does not
require the presence of that subdirectory.
Only those two test cases insist, for some reason that escapes me, on
the presence of those subdirectories.
As I said, Git does not populate that subdirectory. It even lacks all
facilities to populate it. You have to write the files inside it
yourself, you have to figure out the syntax of the files in that
directory, and the only place where we document this is buried deeply in
`Documentation/gitrepository-layout.txt`.
It is a bit silly, too, that we say there, in that very documentation
for that very feature, that this feature is "slightly deprecated".
Either it is deprecated, or it isn't.
Further, it is a bit silly that this "slight deprecation" has been there
since a1d4aa74241 (Add repository-layout document., 2005-09-01).
In other words, that feature was "slightly deprecated" already a mere
five months after Git was made public. Yet we still shlep it around,
fourteen (!) years later.
In yet other words, the support for `.git/branches` has been "slightly
deprecated" more than 33x longer than it hasn't been.
I just wish that my patches to remove the support for `.git/branches`
would have gone somewhere useful instead of into this ugly impasse.
Ciao,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Without this patch, and without removing support for `.git/branches/`,
> you force the Visual Studio build to _not_ pass the test suite. It's as
> simple as that.
I see it as deficiency of the build procedure. What makes it so
different, compared to "make" we type on our boxes, where we do get
the .git/branches/ directory anyway, even without having any file in
it? The patch in question is to punt solving that and instead break
the test suite.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> You probably forgot that I already proposed that, long time ago:
> https://public-inbox.org/git/cover.1494509599.git.johannes.schindelin@gmx.de/
No, I haven't. It was actually meant as an invitation to you to
help us come up with a reasonable deprecation path.
I'd be super happy if we did not have to support 3 sources and
instead just 2 sources of the remote information, but as I said
number of times in the previous attempts' discussion, I think it is
trickier than any of our past migration (like "push.default") to
remove support for .git/branches, as I see no reasonably convenient
alternative/workaround for those who do take advantage of the fact
that .git/branches/ is "one liner, single file per remote source",
which would make it convenient when you need to interact with dozens
of sublevel integrators and the population of them changes
regularly. "Run 'git remote add' with these options to limit its
scope to a single branch" is easy to say but cumbersome to execute.
The best case scenario I came up with is to start giving a message
when we read *and* *use* information from .git/branches/ hierarchy
(i.e. when we know we found a potential user who will get affected
by removal of .git/branches support), asking to tell us not to go
forward with the removal if and only if an alternative we leave for
them gives them unacceptably high cost (i.e. "we cannot afford to
migrate our workflow not to use .git/branches/ because ..."), and
after a few releases we do not hear anything from anybody. The
second best would be to see responses that can serve as concrete
examples to build our easy-to-use alternative after removing the
support for .git/branches/. The alternative might end up not
removing the support, but that is OK---we are in far better position
than we currently are either way. The reason why we still have the
support is mainly because we know there were users who took active
advantage of that facility (again, not Cogito) several years ago,
and we do not know if they moved on to update their workflows to use
the config-based settings. After the above (or something similar)
happens, we will know that nobody needs it and we can remove it with
confidence.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> You probably forgot that I already proposed that, long time ago:
>> https://public-inbox.org/git/cover.1494509599.git.johannes.schindelin@gmx.de/
>
> No, I haven't. It was actually meant as an invitation to you to
> help us come up with a reasonable deprecation path.
By the way, while the "deprecation plan" still has my attention ;-),
there is another thing I've been wanting to see happen *without*
burning me like the last time it was brought up [*1*], which is also
hard to come up with a reasonable deprecation path.
A newer port like Git for Windows, where I suspect that most of the
users are not even aware of the "git-foo" form, can probably get
away by not shipping the libexec/git/git-foo without getting its
users complaining (and as you repeatedly said, nobody on Windows
write shell scripts, so lack of support for old scripts writtin in
the days back when git-foo was a norm is perfectly fine there). But
I am not sure about my tree where audiences are beyond Windows, and
I certainly do not want to get burned again myself.
Somebody else volunteering to take both blame (and flame) and credit
would be most welcomed ;-).
[Reference]
*1* https://public-inbox.org/git/7vr68b8q9p.fsf@gitster.siamese.dyndns.org/
|
This branch is now known as |
|
This patch series was integrated into pu via git@c23712e. |
|
This patch series was integrated into pu via git@1920dd9. |
|
This patch series was integrated into pu via git@36607a4. |
|
This patch series was integrated into pu via git@e2127dc. |
|
This patch series was integrated into pu via git@8fed3fb. |
|
This patch series was integrated into pu via git@64b01f4. |
|
This patch series was integrated into pu via git@f7aefee. |
|
This patch series was integrated into pu via git@6d2eba7. |
Based on the previous patches in this patch series that fixed the generator for `.vcproj` files (which were used by Visual Studio prior to 2015 to define projects), this patch offers to generate project definitions for neweer versions of Visual Studio (which use `.vcxproj` files). To that end, this patch copy-edits the generator of the `.vcproj`. In addition, we now use the `vcpkg` system which allows us to build Git's dependencies (e.g. curl, libexpat) conveniently. The support scripts were introduced in the `jh/msvc` patch series, and with this patch we initialize the `vcpkg` conditionally, in the `libgit` project's `PreBuildEvent`. To allow for parallel building of the projects, we therefore put `libgit` at the bottom of the project hierarchy. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The entire idea of generating the VS solution makes only sense if we generate it via Continuous Integration; otherwise potential users would still have to download the entire Git for Windows SDK. If we pre-generate the Visual Studio solution, Git can be built entirely within Visual Studio, and the test scripts can be run in a regular Git for Windows (e.g. the Portable Git flavor, which does not include a full GCC toolchain and therefore weighs only about a tenth of Git for Windows' SDK). So let's just add a target in the Makefile that can be used to generate said solution; The generated files will then be committed so that they can be pushed to a branch ready to check out by Visual Studio users. To make things even more useful, we also generate and commit other files that are required to run the test suite, such as templates and bin-wrappers: with this, developers can run the test suite in a regular Git Bash after building the solution in Visual Studio. Note: for this build target, we do not actually need to initialize the `vcpkg` system, so we don't. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The default location for `.exe` files linked by Visual Studio depends on the mode (debug vs release) and the architecture. Meaning: after a full build, there is a `git.exe` in the top-level directory, but none of the built-ins are linked.. When running a test script in Git Bash, it therefore would pick up the wrong, say, `git-receive-pack.exe`: the one installed at the same time as the Git Bash. Absolutely not what we want. We want to have confidence that our test covers the MSVC-built Git executables, and not some random stuff. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Add the Microsoft .manifest pattern, and do not anchor the 'Debug' and 'Release' entries at the top-level directory, to allow for multiple projects (one per target). Signed-off-by: Philip Oakley <philipoakley@iee.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When compiling with Visual Studio, the projects' names are identical to the executables modulo the extensions. Read: there will exist both a directory called `git` as well as an executable called `git.exe` in the end. Which means that the bin-wrappers *need* to target the `.exe` files lest they try to execute directories. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is one of the few places where Git violates its own deprecation of the dashed form. It is not necessary, either. As of 595d59e (git.c: ignore pager.* when launching builtin as dashed external, 2017-08-02), Git wants to ignore the pager.* config setting when expanding aliases. So let's strip out the check_pager_config(<command-name>) call from the copy-edited code. This code actually made it into upstream git.git already, but it was disabled in `#if 0 ... #endif` guards so far. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
removed
the
needs-work
|
/submit |
|
Submitted as pull.287.v2.git.gitgitgadget@gmail.com |
|
This patch series was integrated into pu via git@cd5961a. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
| @@ -230,6 +230,7 @@ | |||
| *.ipdb | |||
There was a problem hiding this comment.
On the Git mailing list, SZEDER Gábor wrote (reply to this):
On Mon, Jul 29, 2019 at 01:08:14PM -0700, Philip Oakley via GitGitGadget wrote:
> Add the Microsoft .manifest pattern, and do not anchor the 'Debug'
> and 'Release' entries at the top-level directory, to allow for
> multiple projects (one per target).
>
> Signed-off-by: Philip Oakley <philipoakley@iee.org>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> .gitignore | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index e096e0a51c..e7bb15d301 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -230,6 +230,7 @@
> *.ipdb
> *.dll
> .vs/
> -/Debug/
> -/Release/
> +*.manifest
This new line ignores the tracked file 'compat/win32/git.manifest'
that was added fairly recently in fe90397604 (mingw: embed a manifest
to trick UAC into Doing The Right Thing, 2019-06-27).
I wonder whether that's intentional or accidental.
I'm inclined to think that it's merely accidental, because, as far as
I understand, this is an old-ish patch from times when there wasn't
any 'git.manifest' file in tree, and simply noone noticed that in the
meantime we got one. But I have no idea about how a Git build with
Visual Studio is supposed to work, so it doesn't really matter what
I'm inclined to think :)
There was a problem hiding this comment.
On the Git mailing list, Philip Oakley wrote (reply to this):
Hi Szeder,
On 25/08/2019 13:07, SZEDER Gábor wrote:
> On Mon, Jul 29, 2019 at 01:08:14PM -0700, Philip Oakley via GitGitGadget wrote:
>> Add the Microsoft .manifest pattern, and do not anchor the 'Debug'
>> and 'Release' entries at the top-level directory, to allow for
>> multiple projects (one per target).
>>
>> Signed-off-by: Philip Oakley <philipoakley@iee.org>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>> .gitignore | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/.gitignore b/.gitignore
>> index e096e0a51c..e7bb15d301 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -230,6 +230,7 @@
>> *.ipdb
>> *.dll
>> .vs/
>> -/Debug/
>> -/Release/
>> +*.manifest
> This new line ignores the tracked file 'compat/win32/git.manifest'
> that was added fairly recently in fe90397604 (mingw: embed a manifest
> to trick UAC into Doing The Right Thing, 2019-06-27).
>
> I wonder whether that's intentional or accidental.
>
> I'm inclined to think that it's merely accidental, because, as far as
> I understand, this is an old-ish patch from times when there wasn't
> any 'git.manifest' file in tree, and simply noone noticed that in the
> meantime we got one. But I have no idea about how a Git build with
> Visual Studio is supposed to work, so it doesn't really matter what
> I'm inclined to think :)
>
At the time, it was just one of the many non-source files that were
generated by Visual Studio that cluttered the status list and also could
accidentally added to the tracked files.
The newly added .manifest file does appear to be there to 'trick' the
Windows User Access Control (UAC) which otherwise can be an annoyance to
'regular' users.
Philip
There was a problem hiding this comment.
On the Git mailing list, SZEDER Gábor wrote (reply to this):
On Sun, Aug 25, 2019 at 02:20:32PM +0100, Philip Oakley wrote:
> Hi Szeder,
>
> On 25/08/2019 13:07, SZEDER Gábor wrote:
> >On Mon, Jul 29, 2019 at 01:08:14PM -0700, Philip Oakley via GitGitGadget wrote:
> >>Add the Microsoft .manifest pattern, and do not anchor the 'Debug'
> >>and 'Release' entries at the top-level directory, to allow for
> >>multiple projects (one per target).
> >>
> >>Signed-off-by: Philip Oakley <philipoakley@iee.org>
> >>Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >>---
> >> .gitignore | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/.gitignore b/.gitignore
> >>index e096e0a51c..e7bb15d301 100644
> >>--- a/.gitignore
> >>+++ b/.gitignore
> >>@@ -230,6 +230,7 @@
> >> *.ipdb
> >> *.dll
> >> .vs/
> >>-/Debug/
> >>-/Release/
> >>+*.manifest
> >This new line ignores the tracked file 'compat/win32/git.manifest'
> >that was added fairly recently in fe90397604 (mingw: embed a manifest
> >to trick UAC into Doing The Right Thing, 2019-06-27).
> >
> >I wonder whether that's intentional or accidental.
> >
> >I'm inclined to think that it's merely accidental, because, as far as
> >I understand, this is an old-ish patch from times when there wasn't
> >any 'git.manifest' file in tree, and simply noone noticed that in the
> >meantime we got one. But I have no idea about how a Git build with
> >Visual Studio is supposed to work, so it doesn't really matter what
> >I'm inclined to think :)
> >
> At the time, it was just one of the many non-source files that were
> generated by Visual Studio that cluttered the status list and also could
> accidentally added to the tracked files.
>
> The newly added .manifest file does appear to be there to 'trick' the
> Windows User Access Control (UAC) which otherwise can be an annoyance to
> 'regular' users.
Sorry, I'm not sure how to interpret your reply, and can't decide
whether it tries to justify why that tracked file should be ignored,
or explains that ignoring it was accidental.
Anyway, ignoring that tracked file apparently triggered a nested
worktree-related bug in 'git clean', which can lead to data loss:
https://public-inbox.org/git/20190825185918.3909-1-szeder.dev@gmail.com/
There was a problem hiding this comment.
On the Git mailing list, Philip Oakley wrote (reply to this):
Hi Szeder,
On 25/08/2019 20:09, SZEDER Gábor wrote:
> On Sun, Aug 25, 2019 at 02:20:32PM +0100, Philip Oakley wrote:
>> Hi Szeder,
>>
>> On 25/08/2019 13:07, SZEDER Gábor wrote:
>>> On Mon, Jul 29, 2019 at 01:08:14PM -0700, Philip Oakley via GitGitGadget wrote:
>>>> Add the Microsoft .manifest pattern, and do not anchor the 'Debug'
>>>> and 'Release' entries at the top-level directory, to allow for
>>>> multiple projects (one per target).
>>>>
>>>> Signed-off-by: Philip Oakley <philipoakley@iee.org>
>>>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>>>> ---
>>>> .gitignore | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/.gitignore b/.gitignore
>>>> index e096e0a51c..e7bb15d301 100644
>>>> --- a/.gitignore
>>>> +++ b/.gitignore
>>>> @@ -230,6 +230,7 @@
>>>> *.ipdb
>>>> *.dll
>>>> .vs/
>>>> -/Debug/
>>>> -/Release/
>>>> +*.manifest
>>> This new line ignores the tracked file 'compat/win32/git.manifest'
>>> that was added fairly recently in fe90397604 (mingw: embed a manifest
>>> to trick UAC into Doing The Right Thing, 2019-06-27).
>>>
>>> I wonder whether that's intentional or accidental.
>>>
>>> I'm inclined to think that it's merely accidental, because, as far as
>>> I understand, this is an old-ish patch from times when there wasn't
>>> any 'git.manifest' file in tree, and simply noone noticed that in the
>>> meantime we got one. But I have no idea about how a Git build with
>>> Visual Studio is supposed to work, so it doesn't really matter what
>>> I'm inclined to think :)
>>>
>> At the time, it was just one of the many non-source files that were
>> generated by Visual Studio that cluttered the status list and also could
>> accidentally added to the tracked files.
>>
>> The newly added .manifest file does appear to be there to 'trick' the
>> Windows User Access Control (UAC) which otherwise can be an annoyance to
>> 'regular' users.
> Sorry, I'm not sure how to interpret your reply, and can't decide
> whether it tries to justify why that tracked file should be ignored,
> or explains that ignoring it was accidental.
>
> Anyway, ignoring that tracked file apparently triggered a nested
> worktree-related bug in 'git clean', which can lead to data loss:
>
> https://public-inbox.org/git/20190825185918.3909-1-szeder.dev@gmail.com/
>
Basically manifests are a build artefact from Visual Studio [1], so it
was just another file to be ignored, from a _source_ control control
viewpoint.
I hadn't fully appreciated the reason for your question, but I can now
see how the general 'ignore most, but keep an occasional' file type can
be a problem for the `git clean` filter, along with sub-module repos in
sub-directories. It probably extends beyond this special case .manifest
file.
I doubt that dscho wants to also do file renaming to achieve the trick
that added that file to avoid this. the clean command should never be
ignoring (and possibly deleting) tracked files.
When clean hits a nested repo (maybe with .git directory - not sure of
some of the sub-module changes) then it should stop referring to that
top level gitignore, at least that's my expectation. The commit
description in the referenced patch appears to get the two parts
inter-twined.
Philip
[1]
https://docs.microsoft.com/en-us/cpp/build/manifest-generation-in-visual-studio?view=vs-2019
There was a problem hiding this comment.
On the Git mailing list, SZEDER Gábor wrote (reply to this):
On Sun, Aug 25, 2019 at 11:21:23PM +0100, Philip Oakley wrote:
> >>>>diff --git a/.gitignore b/.gitignore
> >>>>index e096e0a51c..e7bb15d301 100644
> >>>>--- a/.gitignore
> >>>>+++ b/.gitignore
> >>>>@@ -230,6 +230,7 @@
> >>>> *.ipdb
> >>>> *.dll
> >>>> .vs/
> >>>>-/Debug/
> >>>>-/Release/
> >>>>+*.manifest
> >>>This new line ignores the tracked file 'compat/win32/git.manifest'
> >>>that was added fairly recently in fe90397604 (mingw: embed a manifest
> >>>to trick UAC into Doing The Right Thing, 2019-06-27).
> >>>
> >>>I wonder whether that's intentional or accidental.
> >>>
> >>>I'm inclined to think that it's merely accidental, because, as far as
> >>>I understand, this is an old-ish patch from times when there wasn't
> >>>any 'git.manifest' file in tree, and simply noone noticed that in the
> >>>meantime we got one. But I have no idea about how a Git build with
> >>>Visual Studio is supposed to work, so it doesn't really matter what
> >>>I'm inclined to think :)
> >>>
> >>At the time, it was just one of the many non-source files that were
> >>generated by Visual Studio that cluttered the status list and also could
> >>accidentally added to the tracked files.
> >>
> >>The newly added .manifest file does appear to be there to 'trick' the
> >>Windows User Access Control (UAC) which otherwise can be an annoyance to
> >>'regular' users.
> >Sorry, I'm not sure how to interpret your reply, and can't decide
> >whether it tries to justify why that tracked file should be ignored,
> >or explains that ignoring it was accidental.
> >
> >Anyway, ignoring that tracked file apparently triggered a nested
> >worktree-related bug in 'git clean', which can lead to data loss:
> >
> >https://public-inbox.org/git/20190825185918.3909-1-szeder.dev@gmail.com/
> >
> Basically manifests are a build artefact from Visual Studio [1], so it was
> just another file to be ignored, from a _source_ control control viewpoint.
I understand that manifest files, in general, are build artifacts.
But does Visual Studio overwrite the existing
'compat/win32/git.manifest' file in particular? Yes or no? :)
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323328-146101035-1566831445=:46
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hi,
On Sun, 25 Aug 2019, Philip Oakley wrote:
> On 25/08/2019 20:09, SZEDER G=C3=A1bor wrote:
> > On Sun, Aug 25, 2019 at 02:20:32PM +0100, Philip Oakley wrote:
> > >
> > > On 25/08/2019 13:07, SZEDER G=C3=A1bor wrote:
> > > > On Mon, Jul 29, 2019 at 01:08:14PM -0700, Philip Oakley via GitGit=
Gadget
> > > > wrote:
> > > > > Add the Microsoft .manifest pattern, and do not anchor the 'Debu=
g'
> > > > > and 'Release' entries at the top-level directory, to allow for
> > > > > multiple projects (one per target).
> > > > >
> > > > > Signed-off-by: Philip Oakley <philipoakley@iee.org>
> > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > > > ---
> > > > > .gitignore | 5 +++--
> > > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/.gitignore b/.gitignore
> > > > > index e096e0a51c..e7bb15d301 100644
> > > > > --- a/.gitignore
> > > > > +++ b/.gitignore
> > > > > @@ -230,6 +230,7 @@
> > > > > *.ipdb
> > > > > *.dll
> > > > > .vs/
> > > > > -/Debug/
> > > > > -/Release/
> > > > > +*.manifest
> > > > This new line ignores the tracked file 'compat/win32/git.manifest'
> > > > that was added fairly recently in fe90397604 (mingw: embed a manif=
est
> > > > to trick UAC into Doing The Right Thing, 2019-06-27).
> > > >
> > > > I wonder whether that's intentional or accidental.
This was an oversight of mine, my apologies. That line should go.
> > > > I'm inclined to think that it's merely accidental, because, as far=
as
> > > > I understand, this is an old-ish patch from times when there wasn'=
t
> > > > any 'git.manifest' file in tree, and simply noone noticed that in =
the
> > > > meantime we got one. But I have no idea about how a Git build wit=
h
> > > > Visual Studio is supposed to work, so it doesn't really matter wha=
t
> > > > I'm inclined to think :)
> > > >
> > > At the time, it was just one of the many non-source files that were
> > > generated by Visual Studio that cluttered the status list and also c=
ould
> > > accidentally added to the tracked files.
> > >
> > > The newly added .manifest file does appear to be there to 'trick' th=
e
> > > Windows User Access Control (UAC) which otherwise can be an annoyanc=
e to
> > > 'regular' users.
> > Sorry, I'm not sure how to interpret your reply, and can't decide
> > whether it tries to justify why that tracked file should be ignored,
> > or explains that ignoring it was accidental.
> >
> > Anyway, ignoring that tracked file apparently triggered a nested
> > worktree-related bug in 'git clean', which can lead to data loss:
> >
> > https://public-inbox.org/git/20190825185918.3909-1-szeder.dev@gmail.co=
m/
Whoa. At least we found a bug via my oversight ;-)
> Basically manifests are a build artefact from Visual Studio [1], so it w=
as
> just another file to be ignored, from a _source_ control control viewpoi=
nt.
More precisely, manifest files are something specific to Windows, where
you can embed metadata in an executable. Visual Studio auto-generated it
under certain circumstances, but recent versions seem not to do that
anymore.
Ciao,
Dscho
> I hadn't fully appreciated the reason for your question, but I can now s=
ee how
> the general 'ignore most, but keep an occasional' file type can be a pro=
blem
> for the `git clean` filter, along with sub-module repos in sub-directori=
es. It
> probably extends beyond this special case .manifest file.
>
> I doubt that dscho wants to also do file renaming to achieve the trick t=
hat
> added that file to avoid this. the clean command should never be ignorin=
g (and
> possibly deleting) tracked files.
>
> When clean hits a nested repo (maybe with .git directory - not sure of s=
ome of
> the sub-module changes) then it should stop referring to that top level
> gitignore, at least that's my expectation. The commit description in the
> referenced patch appears to get the two parts inter-twined.
>
> Philip
> [1]
> https://docs.microsoft.com/en-us/cpp/build/manifest-generation-in-visual=
-studio?view=3Dvs-2019
>
--8323328-146101035-1566831445=:46--
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > > > > --- a/.gitignore
>> > > > > +++ b/.gitignore
>> > > > > @@ -230,6 +230,7 @@
>> > > > > *.ipdb
>> > > > > *.dll
>> > > > > .vs/
>> > > > > -/Debug/
>> > > > > -/Release/
>> > > > > +*.manifest
>> > > > This new line ignores the tracked file 'compat/win32/git.manifest'
>> > > > that was added fairly recently in fe90397604 (mingw: embed a manifest
>> > > > to trick UAC into Doing The Right Thing, 2019-06-27).
>> > > >
>> > > > I wonder whether that's intentional or accidental.
>
> This was an oversight of mine, my apologies. That line should go.
Good to see people finding glitches in topics that have graduated to
'master' not so long ago. It would have been even nicer if we found
them while in 'next', but we are all human ;-)
> More precisely, manifest files are something specific to Windows, where
> you can embed metadata in an executable. Visual Studio auto-generated it
> under certain circumstances, but recent versions seem not to do that
> anymore.
I expect somebody who knows Windows .manifest better than I do to
come up with a one-liner patch with a single paragraph log message.
Or it can just be a part of a larger series that is the next batch
of Windows updates.
Thanks.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323328-1433146975-1566992067=:46
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hi G=C3=A1bor,
On Mon, 26 Aug 2019, SZEDER G=C3=A1bor wrote:
> On Sun, Aug 25, 2019 at 11:21:23PM +0100, Philip Oakley wrote:
> > >>>>diff --git a/.gitignore b/.gitignore
> > >>>>index e096e0a51c..e7bb15d301 100644
> > >>>>--- a/.gitignore
> > >>>>+++ b/.gitignore
> > >>>>@@ -230,6 +230,7 @@
> > >>>> *.ipdb
> > >>>> *.dll
> > >>>> .vs/
> > >>>>-/Debug/
> > >>>>-/Release/
> > >>>>+*.manifest
> > >>>This new line ignores the tracked file 'compat/win32/git.manifest'
> > >>>that was added fairly recently in fe90397604 (mingw: embed a manife=
st
> > >>>to trick UAC into Doing The Right Thing, 2019-06-27).
> > >>>
> > >>>I wonder whether that's intentional or accidental.
> > >>>
> > >>>I'm inclined to think that it's merely accidental, because, as far =
as
> > >>>I understand, this is an old-ish patch from times when there wasn't
> > >>>any 'git.manifest' file in tree, and simply noone noticed that in t=
he
> > >>>meantime we got one. But I have no idea about how a Git build with
> > >>>Visual Studio is supposed to work, so it doesn't really matter what
> > >>>I'm inclined to think :)
> > >>>
> > >>At the time, it was just one of the many non-source files that were
> > >>generated by Visual Studio that cluttered the status list and also c=
ould
> > >>accidentally added to the tracked files.
> > >>
> > >>The newly added .manifest file does appear to be there to 'trick' th=
e
> > >>Windows User Access Control (UAC) which otherwise can be an annoyanc=
e to
> > >>'regular' users.
> > >Sorry, I'm not sure how to interpret your reply, and can't decide
> > >whether it tries to justify why that tracked file should be ignored,
> > >or explains that ignoring it was accidental.
> > >
> > >Anyway, ignoring that tracked file apparently triggered a nested
> > >worktree-related bug in 'git clean', which can lead to data loss:
> > >
> > >https://public-inbox.org/git/20190825185918.3909-1-szeder.dev@gmail.c=
om/
> > >
> > Basically manifests are a build artefact from Visual Studio [1], so it=
was
> > just another file to be ignored, from a _source_ control control viewp=
oint.
>
> I understand that manifest files, in general, are build artifacts.
> But does Visual Studio overwrite the existing
> 'compat/win32/git.manifest' file in particular? Yes or no? :)
No.
The reason this entry was there: at least _some_ Visual Studio versions
(IIRC), auto-generates `.manifest` files when the project does not have
any. But now we do. So this line's gotta go.
#leftoverbits ?
Ciao,
Dscho
--8323328-1433146975-1566992067=:46--
|
This patch series was merged into |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long time ago, we added support to build
.slnand.vcprojfiles for use within Visual Studio, so that Git could be built in that popular IDE.This support languished for years and was finally brought back into Git for Windows partially through the
jh/msvcbranch that was just merged intomaster. With the tremendous help by Philip Oakley and Jeff Hostetler, this re-adds support for Visual Studio, focusing on VS 2015 and later (older versions are currently unsupported, even if we worked a bit on that front, too; If there are volunteers to add support for older versions, I would be delighted to review PRs to that end on https://github.com/git-for-windows/git).Note: the currently preferred way to contribute using Visual Studio is by using Git for Windows'
vs/masterbranch (which is automatically generated using themake vcxprojbuild target that this patch series adds, updated every time Git for Windows'masteradvances). My plan is to provide a similar branch in GitGitGadget, based on top of git.git'smaster, once this patch series is integrated.Changes since v1:
templates/blt/branches/is now created as part of the build.