Skip to content

Conversation

@dscho
Copy link
Member

@dscho dscho commented Nov 21, 2022

I was about to write up guidelines how to write this patch, but it turned out that it was much faster to write the patch instead.

dscho and others added 30 commits October 7, 2022 09:38
If `feature.experimental` and `feature.manyFiles` are set and the user
has not explicitly turned off the builtin FSMonitor, we now start
the built-in FSMonitor by default.

Only forcing it when UNSET matches the behavior of UPDATE_DEFAULT_BOOL()
used for other repo settings.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Jeff Hostetler <[email protected]>
To verify that the `clean` side of the `clean`/`smudge` filter code is
correct with regards to LLP64 (read: to ensure that `size_t` is used
instead of `unsigned long`), here is a test case using a trivial filter,
specifically _not_ writing anything to the object store to limit the
scope of the test case.

As in previous commits, the `big` file from previous test cases is
reused if available, to save setup time, otherwise re-generated.

Signed-off-by: Philip Oakley <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
This is another fall-out of the recent refactoring flurry.

Signed-off-by: Johannes Schindelin <[email protected]>
This fixes the build after 7bc341e (git-compat-util: add a test
balloon for C99 support, 2021-12-01).

Signed-off-by: Johannes Schindelin <[email protected]>
This is now passed by default, ever since 6a8cbc4 (developer: enable
pedantic by default, 2021-09-03).

Signed-off-by: Johannes Schindelin <[email protected]>
Since ef8a6c6 (reftable: utility functions, 2021-10-07) we not only
have a libreftable, but also a libreftable_test.

Signed-off-by: Johannes Schindelin <[email protected]>
Visual Studio 2022 does not like that at all.

Signed-off-by: Johannes Schindelin <[email protected]>
These refactorings are really gifts that keep on giving.

Signed-off-by: Johannes Schindelin <[email protected]>
In the case of Git for Windows (say, in a Git Bash window) running in a
Windows Subsystem for Linux (WSL) directory, the GetNamedSecurityInfoW()
call in is_path_owned_By_current_side() returns an error code other than
ERROR_SUCCESS. This is consistent behavior across this boundary.

In these cases, the owner would always be different because the WSL
owner is a different entity than the Windows user.

The change here is to suppress the error message that looks like this:

  error: failed to get owner for '//wsl.localhost/...' (1)

Before this change, this warning happens for every Git command,
regardless of whether the directory is marked with safe.directory.

Signed-off-by: Derrick Stolee <[email protected]>
For Windows builds >= 15063 set $env:TERM to "xterm-256color" instead of
"cygwin" because they have a more capable console system that supports
this. Also set $env:COLORTERM="truecolor" if unset.

$env:TERM is initialized so that ANSI colors in color.c work, see
29a3963 (Win32: patch Windows environment on startup, 2012-01-15).

See git-for-windows#3629 regarding problems caused by always setting
$env:TERM="cygwin".

This is the same heuristic used by the Cygwin runtime.

Signed-off-by: Rafael Kitover <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
NtQueryObject under Wine can return a success but fill out no name.
In those situations, Wine will set Buffer to NULL, and set result to
the sizeof(OBJECT_NAME_INFORMATION).

Running a command such as

echo "$(git.exe --version 2>/dev/null)"

will crash due to a NULL pointer dereference when the code attempts to
null terminate the buffer, although, weirdly, removing the subshell or
redirecting stdout to a file will not trigger the crash.

Code has been added to also check Buffer and Length to ensure the check
is as robust as possible due to the current behavior being fragile at
best, and could potentially change in the future

This code is based on the behavior of NtQueryObject under wine and
reactos.

Signed-off-by: Christopher Degawa <[email protected]>
Atomic append on windows is only supported on local disk files, and it may
cause errors in other situations, e.g. network file system. If that is the
case, this config option should be used to turn atomic append off.

Co-Authored-By: Johannes Schindelin <[email protected]>
Signed-off-by: 孙卓识 <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
It is not useful because we do not have any persisted directory anymore,
not since dropping our Travis CI support.

Signed-off-by: Johannes Schindelin <[email protected]>
... so that we can test a MinGit backport in a private repository (with
GitHub Actions, minutes and parallel jobs are limited way more than with
Azure Pipelines in private repositories).

In this commit, we reinstate the exact version of `azure-pipelines.yml`
as 6081d38 (ci: retire the Azure Pipelines definition, 2020-04-11)
deleted.

Naturally, many adjustments are required to make it work again. Some of
the changes are actually outside of that file (such as the
`runs_on_pool` changes that are needed in the Azure Pipelines part of
`ci/lib.sh`) and they were made in the commits leading up to this here
commit.

However, other adjustments are required in the `azure-pipelines.yml`
file itself, and for ease of review (read: to build confidence in those
changes) they will be made in subsequent, individual commits that
explain the intent, context, implementation and justification like every
good commit message should do.

Signed-off-by: Johannes Schindelin <[email protected]>
This is a follow-up to 6c280b4 (ci: remove GETTEXT_POISON jobs,
2021-01-20) after reinstating the Azure Pipeline.

Signed-off-by: Johannes Schindelin <[email protected]>
We have `ci/install-dependencies.sh` for that. Incidentally, this avoids
the following error in the linux-* jobs:

    The following packages have unmet dependencies:
    git-email : Depends: git (< 1:2.25.1-.) but 1:2.35.1-0ppa1~ubuntu20.04.1 is to be installed
	  Recommends: libemail-valid-perl but it is not going to be installed

Signed-off-by: Johannes Schindelin <[email protected]>
We haven't used this feature in ages, we don't actually need to.

Signed-off-by: Johannes Schindelin <[email protected]>
The mingw-w64 GCC seems to link implicitly to libwinpthread, which does
implement a pthread emulation (that is more complete than Git's). Let's
keep preferring Git's.

To avoid linker errors where it thinks that the `pthread_self` and the
`pthread_create` symbols are defined twice, let's give our version a
`win32_` prefix, just like we already do for `pthread_join()`.

Signed-off-by: Johannes Schindelin <[email protected]>
…imal-sdk

The Azure Pipeline `git-sdk-64-minimal` was retired...

Signed-off-by: Johannes Schindelin <[email protected]>
We are about to vendor in `mimalloc`'s source code which we will want to
include `git-compat-util.h` after defining that constant.

Signed-off-by: Johannes Schindelin <[email protected]>
These many refactorings in Git sure are gifts that keep on giving.

Signed-off-by: Johannes Schindelin <[email protected]>
This commit imports mimalloc's source code as per v2.0.6, fetched from
the tag at https://github.com/microsoft/mimalloc.

The .c files are from the src/ subdirectory, and the .h files from the
include/ subdirectory. We will subsequently modify the source code to
accommodate building within Git's context.

Since we plan on using the `mi_*()` family of functions, we skip the
C++-specific source code, some POSIX compliant functions to interact
with mimalloc, and the code that wants to support auto-magic overriding
of the `malloc()` function (mimalloc-new-delete.h, alloc-posix.c,
mimalloc-override.h, alloc-override.c, alloc-override-osx.c,
alloc-override-win.c and static.c).

To appease the `check-whitespace` job of Git's Continuous Integration,
this commit was washed one time via `git rebase --whitespace=fix`.

Signed-off-by: Johannes Schindelin <[email protected]>
This is a backport of d051ed7 (.github/workflows/main.yml: run
static-analysis on bionic, 2021-02-08) to the Azure Pipeline.

When Azure Pipelines' build agents transitioned 'ubuntu-latest' from
18.04 to 20.04, it broke our `static-analysis` job, since Coccinelle
isn't available on Ubuntu focal (it is only available in the universe
suite).

Until Coccinelle can be installed from 20.04's main suite, pin the
static-analysis build to run on 18.04, where it can be installed by
default.

Signed-off-by: Taylor Blau <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Whith Windows 2000, Microsoft introduced a flag to the PE header to mark executables as
"terminal server aware". Windows terminal servers provide a redirected Windows directory and
redirected registry hives when launching legacy applications without this flag set. Since we
do not use any INI files in the Windows directory and don't write to the registry, we don't
need  this additional preparation. Telling the OS that we don't need this should provide
slightly improved startup times in terminal server environments.

When building for supported Windows Versions with MSVC the /TSAWARE linker flag is
automatically set, but MinGW requires us to set the --tsaware flag manually.

This partially addresses git-for-windows#3935.

Signed-off-by: Matthias Aßhauer <[email protected]>
We want to compile mimalloc's source code as part of Git, rather than
requiring the code to be built as an external library: mimalloc uses a
CMake-based build, which is not necessarily easy to integrate into the
flavors of Git for Windows (which will be the main benefitting port).

Signed-off-by: Johannes Schindelin <[email protected]>
This format is not supported by MSVC runtimes targeted by the MINGW
toolchain.

Signed-off-by: Johannes Schindelin <[email protected]>
Instead, load the `GetProcessMemoryInfo()` function dynamically. When
needed. If needed.

This is necessary because the start-up cost of Git processes spent on
loading dynamic libraries is non-negligible.

Signed-off-by: Johannes Schindelin <[email protected]>
Instead, load the `BCryptGenRandom()` function dynamically. When
needed. If needed.

This is necessary because the start-up cost of Git processes spent on
loading dynamic libraries is non-negligible.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho and others added 25 commits October 7, 2022 09:52
azure-pipeline: run static-analysis on jammy

This is inspired by d051ed7 (.github/workflows/main.yml: run
static-analysis on bionic, 2021-02-08) and by ef46584 (ci: update
'static-analysis' to Ubuntu 22.04, 2022-08-23), adapted to the Azure
Pipeline.

When Azure Pipelines' build agents transitioned 'ubuntu-latest' from
18.04 to 20.04, it broke our `static-analysis` job, since Coccinelle
was not madeavailable on Ubuntu focal (it is only available in the
universe suite).

This is not an issue with Ubuntu 22.04, but we will only know whether it
is an issue with 24.04 when _that_ comes out. So let's play it safe and
pin the `static_analysis` job to the latest Ubuntu version that we know
to offer a working Coccinelle package.

Signed-off-by: Taylor Blau <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
…rge deltas

We need to explicitly allow the `file://` protocol for our tests to
work, as a consequence of the fix for CVE-2022-39253.

Signed-off-by: Johannes Schindelin <[email protected]>
Cherry pick commit d3775de (Makefile: force -O0 when compiling with
SANITIZE=leak, 2022-10-18), as otherwise the leak checker at GitHub
Actions CI seems to fail with a false positive.

[Backported to v2.37.0 for easier merging e.g. into Git for Windows]

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
The CI builds have been seriously broken as of v2.38.1 by the constantly
failing `linux-leaks` job. Let's backport the work-around that has been
applied to upstream Git (but that funnily has not been based directly on
top of v2.38.1).
When a test script fails in Git's test suite, the usual course of action
is to re-run it using options to increase the verbosity of the output,
e.g. `-v` and `-x`.

Like in Git's CI runs, when running the tests in Visual Studio via the
CTest route, it is cumbersome or at least requires a very unintuitive
approach to pass options to the test scripts: the CMakeLists.txt file
would have to be modified, passing the desired options to _all_ test
scripts, and then the CMake Cache would have to be reconfigured before
running the test in question individually. Unintuitive at best, and
opposite to the niceties IDE users expect.

So let's just pass those options by default: This will not clutter any
output window but the log that is written to a log file will have
information necessary to figure out test failures.

While at it, also imitate what the Windows jobs in Git's CI runs do to
accelerate running the test scripts: pass the `--no-bin-wrappers` and
`--no-chain-lint` options.

This makes the test runs noticeably faster because the `bin-wrappers/`
scripts as well as the `chain-lint` code make heavy use of POSIX shell
scripting, which is really, really slow on Windows due to the need to
emulate POSIX behavior via the MSYS2 runtime. In a test by Eric
Sunshine, it added two minutes (!) just to perform the chain-lint task.

The idea of adding a CMake config option (á la `GIT_TEST_OPTS`) was
considered during the development of this patch, but then dropped: such
a setting is global, across _all_ tests, where e.g. `--run=...` would
not make sense. Users wishing to override these new defaults are better
advised running the test script manually, in a Git Bash, with full
control over the command line.

Signed-off-by: Johannes Schindelin <[email protected]>
Even when running the tests via CTest, t7609 and t7610 rely on more than
only a few mergetools to be copied to the build directory. Let's make it
so.

Signed-off-by: Johannes Schindelin <[email protected]>
In the interactive `add` operation, users can choose to jump to specific
hunks, and Git will present the hunk list in that case. To avoid showing
too many lines at once, only a maximum of 21 hunks are shown, skipping
the "mode change" pseudo hunk.

The comparison performed to skip the "mode change" pseudo hunk (if any)
compares a signed integer `i` to the unsigned value `mode_change` (which
can be 0 or 1 because it is a 1-bit type).

According to section 6.3.1.8 of the C99 standard (see e.g.
https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
happen is an automatic conversion of the "lesser" type to the "greater"
type, but since the types differ in signedness, it is ill-defined what
is the correct "usual arithmetic conversion".

Which means that Visual C's behavior can (and does) differ from GCC's:
When compiling Git using the latter, `add -p`'s `goto` command shows no
hunks by default because it casts a negative start offset to a pretty
large unsigned value, breaking the "goto hunk" test case in
`t3701-add-interactive.sh`.

Let's avoid that by converting the unsigned bit explicitly to a signed
integer.

Note: This is a long-standing bug in the Visual C build of Git, but it
has never been caught because t3701 is skipped when `NO_PERL` is set,
which is the case in the `vs-test` jobs of Git's CI runs.

Signed-off-by: Johannes Schindelin <[email protected]>
In 7f5397a (cmake: support for testing git when building out of the
source tree, 2020-06-26), we implemented support for running Git's test
scripts even after building Git in a different directory than the source
directory.

The way we did this was to edit the file `t/test-lib.sh` to override
`GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
directory.

This is unideal because it always leaves a tracked file marked as
modified, and it is all too easy to commit that change by mistake.

Let's change the strategy by teaching `t/test-lib.sh` to detect the
presence of a file called `GIT-BUILD-DIR` in the source directory. If it
exists, the contents are interpreted as the location to the _actual_
build directory. We then write this file as part of the CTest
definition.

To support building Git via a regular `make` invocation after building
it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
convenience, this is done as part of the Makefile rule that is already
run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
up to date).

Signed-off-by: Johannes Schindelin <[email protected]>
As suggested in
git-for-windows#3966 (comment),
t7112 can run for well over one hour, which seems to be the default
maximum run time at least when running CTest-based tests in Visual
Studio.

Let's increase the time-out as a stop gap to unblock developers wishing
to run Git's test suite in Visual Studio.

Note: The actual run time is highly dependent on the circumstances. For
example, in Git's CI runs, the Windows-based tests typically take a bit
over 5 minutes to run. CI runs have the added benefit that Windows
Defender (the common anti-malware scanner on Windows) is turned off,
something many developers are not at liberty to do on their work
stations. When Defender is turned on, even on this developer's high-end
Ryzen system, t7112 takes over 15 minutes to run.

Signed-off-by: Johannes Schindelin <[email protected]>
In 7f5397a (cmake: support for testing git when building out of the
source tree, 2020-06-26), we implemented support for running Git's test
scripts even after building Git in a different directory than the source
directory.

The way we did this was to edit the file `t/test-lib.sh` to override
`GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
directory.

This is unideal because it always leaves a tracked file marked as
modified, and it is all too easy to commit that change by mistake.

Let's change the strategy by teaching `t/test-lib.sh` to detect the
presence of a file called `GIT-BUILD-DIR` in the source directory. If it
exists, the contents are interpreted as the location to the _actual_
build directory. We then write this file as part of the CTest
definition.

To support building Git via a regular `make` invocation after building
it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
convenience, this is done as part of the Makefile rule that is already
run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
up to date).

Signed-off-by: Johannes Schindelin <[email protected]>
This is a backport of the patches that made it into core Git via
246eedf (Merge branch 'js/cmake-updates', 2022-10-27).

Signed-off-by: Johannes Schindelin <[email protected]>
When building Git via CMake on Windows, the dynamic libraries (`.dll`
files) on which Git's executables depend are copied into the build
directory, because the `.exe` files would not even load otherwise.

The "'MSYSTEM/PATH is adjusted if necessary'" test case of
`t0060-path-utils.sh` wants to copy said `.exe` files, though, and
expects them to run. Therefore, we need to copy the `.dll` files, too,
if there are any.

Signed-off-by: Johannes Schindelin <[email protected]>
Newer compiler versions, like GCC 10 and Clang 12, have built-in
functions for bswap32 and bswap64. This comes in handy, for example,
when targeting CLANGARM64 on Windows, which would not be supported
without this logic.

Signed-off-by: Dennis Ameling <[email protected]>
CLANGARM64 is a relatively new MSYSTEM added by the MSYS2 team. In order
to have Git build correctly for this platform, let's add some
configuration for it to config.mak.uname.

Signed-off-by: Dennis Ameling <[email protected]>
No GitHub-hosted ARM64 runners are available at the moment of writing,
but folks can leverage self-hosted runners of this architecture. This CI
pipeline comes in handy for forks of the git-for-windows/git project
that have such runners available. The pipeline can be kicked off
manually through a workflow_dispatch.

Signed-off-by: Dennis Ameling <[email protected]>
**This requires an ARM64-machine with Windows 11 installed (which
supports x64 emulation for MSYS2)**

### The main idea

- Use the main MSYS2/git-sdk-64 setup, which works on Windows 11 on ARM
thanks to x64-emulation
- Configure the official `clangarm64` MSYS2 repo
- Install `mingw-w64-clang-aarch64-toolchain` which contains the
ARM64-native Clang compiler
As we've discovered with Git 2.37.1, our workflow to inform us about component
updates can miss embargoed git releases, since GitHub usues the tag creation
date as the publication date for their feeds. To prevent similar issues with
other components in the future, let's increase the time span we look at for
new feed items, as discussed in
git-for-windows#3948 (comment)

Signed-off-by: Matthias Aßhauer <[email protected]>
The `environment` keyword makes GitHub Actions believe this is a deployment
environment [1], but we aren't deploying anything in this workflow. Having
a "deployment" workflow on a frequent schedule causes some pull requests like
[2] to become filled with thousands of "dscho deployed temporarily to
monitor-components" messages. This was originally intended to allow this
workflow to open issues, but we can instead just specify that the workflow
should run with permissions to create issues and then use the generated
access token.

[1] https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment
[2] git-for-windows#3948

Signed-off-by: Matthias Aßhauer <[email protected]>
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes git-for-windows#3966
The `GetNumaProcessorNode()` symbol is not defined in Nano Server's DLLs
(because that function is long deprecated). This caused Git no longer to
work on Nano Server.

Instead of importing it statically, try to import it dynamically, and
fall back gracefully if it cannot be loaded.

This fixes git-for-windows#4052

Signed-off-by: Johannes Schindelin <[email protected]>
Add FileVersion, which is a required field
As not all required fields were present, none were being included
Fixes git-for-windows#4090

Signed-off-by: Kiel Hurley <[email protected]>
Add `FileVersion`, which is a required string ([Microsoft
documentation](https://learn.microsoft.com/en-us/windows/win32/menurc/versioninfo-resource))
in the `StringFileInfo` block.
As not all required strings were present in the block, none were being
included.
Fixes git-for-windows#4090

After including the `FileVersion` string, all other defined strings are
now being included on executables.

File version information for `git.exe` has changed from:
```
PS C:\Program Files\Git\bin> [System.Diagnostics.FileVersionInfo]::GetVersionInfo("C:\Data\git-sdk-64\usr\src\git\git.exe") | Select-Object *

FileVersionRaw     : 2.38.1.1
ProductVersionRaw  : 2.38.1.1
Comments           :
CompanyName        :
FileBuildPart      : 1
FileDescription    :
FileMajorPart      : 2
FileMinorPart      : 38
FileName           : C:\Data\git-sdk-64\usr\src\git\git.exe
FilePrivatePart    : 1
FileVersion        :
InternalName       :
IsDebug            : False
IsPatched          : False
IsPrivateBuild     : False
IsPreRelease       : False
IsSpecialBuild     : False
Language           : English (United States)
LegalCopyright     :
LegalTrademarks    :
OriginalFilename   :
PrivateBuild       :
ProductBuildPart   : 1
ProductMajorPart   : 2
ProductMinorPart   : 38
ProductName        :
ProductPrivatePart : 1
ProductVersion     :
SpecialBuild       :
```

To the following:
```
PS C:\Program Files\Git\bin> [System.Diagnostics.FileVersionInfo]::GetVersionInfo("C:\Data\git-sdk-64\usr\src\git\git.exe") | Select-Object *

FileVersionRaw     : 2.38.1.1
ProductVersionRaw  : 2.38.1.1
Comments           :
CompanyName        : The Git Development Community
FileBuildPart      : 1
FileDescription    : Git for Windows
FileMajorPart      : 2
FileMinorPart      : 38
FileName           : C:\Data\git-sdk-64\usr\src\git\git.exe
FilePrivatePart    : 1
FileVersion        : 2.38.1.windows.1.10.g6ed65a6fab
InternalName       : git
IsDebug            : False
IsPatched          : False
IsPrivateBuild     : False
IsPreRelease       : False
IsSpecialBuild     : False
Language           : English (United States)
LegalCopyright     :
LegalTrademarks    :
OriginalFilename   : git.exe
PrivateBuild       :
ProductBuildPart   : 1
ProductMajorPart   : 2
ProductMinorPart   : 38
ProductName        : Git
ProductPrivatePart : 1
ProductVersion     : 2.38.1.windows.1.10.g6ed65a6fab
SpecialBuild       :
```

I wasn't really expecting `GIT_VERSION` to contain the Git commit, I was
hoping for just `2.38.1` or `2.38.1.1`, at least for the `FileVersion`
string.

Anybody know if it's possible to concatenate the `MAJOR`, `MINOR`,
`MICRO`, and `PATCHLEVEL` fields with dots, or if there's another
variable that can be used (with or without `PATCHLEVEL`)?
Alternatively, use the complete `GIT_VERSION` for both `FileVersion` and
`ProductVersion`.
…ndows#4074)

The `GetNumaProcessorNode()` symbol is not defined in Nano Server's DLLs 
(because that function is long deprecated). This caused Git no longer to
work on Nano Server.

Instead of importing it statically, try to import it dynamically, and
fall back gracefully if it cannot be loaded.

This fixes git-for-windows#4052
We also need to monitor libfido2 (and its dependency libcbor) because
that library is responsible for support of security keys in OpenSSH,
e.g. making it work with Windows Hello.
@dscho
Copy link
Member Author

dscho commented Nov 21, 2022

Sorry, I fat-fingered the source branch name and mistakenly used the main branch. Sadly, PRs do not allow to choose another PR branch name, so I had to close this PR and open #1428 instead.

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

Labels

None yet