git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Carlo Arenas" <carenas@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Johannes Schindelin" <johannes.schindelin@gmail.com>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: [PATCH v4 0/8] Allow clean/smudge filters to handle huge files in the LLP64 data model
Date: Tue, 02 Nov 2021 15:46:03 +0000	[thread overview]
Message-ID: <pull.1068.v4.git.1635867971.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1068.v3.git.1635515959.gitgitgadget@gmail.com>

This patch series came in via the Git for Windows fork
[https://github.com/git-for-windows/git/pull/3487], and I intend to merge it
before v2.34.0-rc0, therefore I appreciate every careful review you gentle
people can spare.

The x86_64 variant of Windows uses the LLP64 data model, where the long data
type is 32-bit. This is very different from the LP64 data model used e.g. by
x86_64 Linux, where unsigned long is 64-bit.

Most notably, this means that sizeof(unsigned long) != sizeof(size_t) in
general.

However, since Git was born in the Linux ecosystem, where that inequality
does not hold true, it is understandable that unsigned long is used in many
code locations where size_t should have been used. As a consequence, quite a
few things are broken e.g. on Windows, when it comes to 4GB file contents or
larger.

Using Git LFS [https://git-lfs.github.io/] trying to work around such issues
is one such a broken scenario. You cannot git checkout, say, 5GB files. Huge
files will be truncated to whatever the file size is modulo 4GB (in the case
of a 5GB file, it would be truncated to 1GB).

This patch series primarily fixes the Git LFS scenario, by allowing clean
filters to accept 5GB files, and by allowing smudge filters to produce 5GB
files.

The much larger project to teach Git to use size_t instead of unsigned long
in all the appropriate places is hardly scratched by this patch series.

Side note: The fix for the clean filter included in this series does not
actually affect Git LFS! The reason is that Git LFS marks its filter as
required, and therefore Git streams the file contents to Git LFS via a file
descriptor (which is unaffected by LLP64). A "clean" filter that is not
marked as required, however, lets Git take the code path that is fixed by
this patch series.

Changes since v3:

 * The commit message of the fourth patch no longer talks about a "workdir"
   but about the "working tree".
 * The smudge filter test case is now more precise when verifying the
   result's file size.

Changes since v2:

 * The test cases now use a prereq to avoid running in 32-bit setups (where
   they would be guaranteed to fail).
 * The SIZE_T_IS64BIT prereq specifically does not follow LONG_IS_64BIT, by
   testing for exactly 64 bits.
 * The code comment above unsigned_left_shift_overflows() was fixed.
 * unsigned_left_shift_overflows() now verifies that shift is smaller than
   the bit size of the operand.
 * genzeros now marks its buffer of NULs as const.
 * The error check in the infinite loop genzeros was fixed.

Changes since v1:

 * Removed extraneous "Signed-off-by:" lines from "git-compat-util:
   introduce more size_t helpers".
 * Integrated Carlo's patch to allow genzeros to generate large amounts of
   NULs, even in LLP64 data models.
 * Using test-tool genzeros instead of dd if=/dev/zero, to help HP NonStop
   (which appears to use the LP64 data model and therefore should pass the
   new test cases even without the fixes provided in this patch series).
 * Accelerating genzeros to have performance characteristics similar to dd
   if=/dev/zero instead of being ~50x slower.

Carlo Marcelo Arenas Belón (2):
  test-genzeros: allow more than 2G zeros in Windows
  test-lib: add prerequisite for 64-bit platforms

Johannes Schindelin (2):
  test-tool genzeros: generate large amounts of data more efficiently
  git-compat-util: introduce more size_t helpers

Matt Cooper (4):
  t1051: introduce a smudge filter test for extremely large files
  odb: teach read_blob_entry to use size_t
  odb: guard against data loss checking out a huge file
  clean/smudge: allow clean filters to process extremely large files

 convert.c                   |  2 +-
 delta.h                     |  6 +++---
 entry.c                     |  8 +++++---
 entry.h                     |  2 +-
 git-compat-util.h           | 25 +++++++++++++++++++++++++
 object-file.c               |  6 +++---
 packfile.c                  |  6 +++---
 parallel-checkout.c         |  2 +-
 t/helper/test-genzeros.c    | 21 +++++++++++++++++----
 t/t1051-large-conversion.sh | 26 ++++++++++++++++++++++++++
 t/test-lib.sh               |  4 ++++
 11 files changed, 89 insertions(+), 19 deletions(-)


base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1068%2Fdscho%2Fhuge-file-smudge-clean-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1068/dscho/huge-file-smudge-clean-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1068

Range-diff vs v3:

 1:  068f897b973 = 1:  068f897b973 test-genzeros: allow more than 2G zeros in Windows
 2:  05219720014 = 2:  05219720014 test-tool genzeros: generate large amounts of data more efficiently
 3:  489500bb1dc = 3:  489500bb1dc test-lib: add prerequisite for 64-bit platforms
 4:  ce9dfaac9f8 ! 4:  f099b4eaead t1051: introduce a smudge filter test for extremely large files
     @@ Commit message
          t1051: introduce a smudge filter test for extremely large files
      
          The filter system allows for alterations to file contents when they're
     -    added to the database or workdir. ("Smudge" when moving to the workdir;
     -    "clean" when moving to the database.) This is used natively to handle CRLF
     -    to LF conversions. It's also employed by Git-LFS to replace large files
     -    from the workdir with small tracking files in the repo and vice versa.
     +    added to the database or working tree. ("Smudge" when moving to the
     +    working tree; "clean" when moving to the database.) This is used
     +    natively to handle CRLF to LF conversions. It's also employed by Git-LFS
     +    to replace large files from the working tree with small tracking files
     +    in the repo and vice versa.
      
     -    Git pulls the entire smudged file into memory. While this is inefficient,
     -    there's a more insidious problem on some platforms due to inconsistency
     -    between using unsigned long and size_t for the same type of data (size of
     -    a file in bytes). On most 64-bit platforms, unsigned long is 64 bits, and
     -    size_t is typedef'd to unsigned long. On Windows, however, unsigned long is
     -    only 32 bits (and therefore on 64-bit Windows, size_t is typedef'd to
     +    Git reads the entire smudged file into memory to convert it into a
     +    "clean" form to be used in-core. While this is inefficient, there's a
     +    more insidious problem on some platforms due to inconsistency between
     +    using unsigned long and size_t for the same type of data (size of a file
     +    in bytes). On most 64-bit platforms, unsigned long is 64 bits, and
     +    size_t is typedef'd to unsigned long. On Windows, however, unsigned long
     +    is only 32 bits (and therefore on 64-bit Windows, size_t is typedef'd to
          unsigned long long in order to be 64 bits).
      
          Practically speaking, this means 64-bit Windows users of Git-LFS can't
     @@ t/t1051-large-conversion.sh: test_expect_success 'ident converts on output' '
      +test_expect_failure EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
      +		'files over 4GB convert on output' '
      +	test_commit test small "a small file" &&
     ++	small_size=$(test_file_size small) &&
      +	test_config filter.makelarge.smudge \
      +		"test-tool genzeros $((5*1024*1024*1024)) && cat" &&
      +	echo "small filter=makelarge" >.gitattributes &&
      +	rm small &&
      +	git checkout -- small &&
      +	size=$(test_file_size small) &&
     -+	test "$size" -ge $((5 * 1024 * 1024 * 1024))
     ++	test "$size" -eq $((5 * 1024 * 1024 * 1024 + $small_size))
      +'
      +
       test_done
 5:  dbef8168bc7 ! 5:  308a8f2a3ad odb: teach read_blob_entry to use size_t
     @@ t/t1051-large-conversion.sh: test_expect_success 'ident converts on output' '
      +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
       		'files over 4GB convert on output' '
       	test_commit test small "a small file" &&
     - 	test_config filter.makelarge.smudge \
     + 	small_size=$(test_file_size small) &&
 6:  18419070c29 = 6:  65bc291b680 git-compat-util: introduce more size_t helpers
 7:  f59c523bcc4 = 7:  7b6655f03f5 odb: guard against data loss checking out a huge file
 8:  acc5591517f ! 8:  41fda423982 clean/smudge: allow clean filters to process extremely large files
     @@ convert.c: static int crlf_to_worktree(const char *src, size_t len, struct strbu
      
       ## t/t1051-large-conversion.sh ##
      @@ t/t1051-large-conversion.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
     - 	test "$size" -ge $((5 * 1024 * 1024 * 1024))
     + 	test "$size" -eq $((5 * 1024 * 1024 * 1024 + $small_size))
       '
       
      +# This clean filter writes down the size of input it receives. By checking against

-- 
gitgitgadget

  parent reply	other threads:[~2021-11-02 15:46 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-27  7:49 [PATCH 0/5] Allow clean/smudge filters to handle huge files in the LLP64 data model Johannes Schindelin via GitGitGadget
2021-10-27  7:49 ` [PATCH 1/5] t1051: introduce a smudge filter test for extremely large files Matt Cooper via GitGitGadget
2021-10-28  7:15   ` Carlo Arenas
2021-10-28  8:54     ` [PATCH] helper/test-genzeros: allow more than 2G zeros in Windows Carlo Marcelo Arenas Belón
2021-10-28 20:32       ` Johannes Schindelin
2021-10-27  7:49 ` [PATCH 2/5] odb: teach read_blob_entry to use size_t Matt Cooper via GitGitGadget
2021-10-27  7:49 ` [PATCH 3/5] git-compat-util: introduce more size_t helpers Johannes Schindelin via GitGitGadget
2021-10-27  7:49 ` [PATCH 4/5] odb: guard against data loss checking out a huge file Matt Cooper via GitGitGadget
2021-10-27  7:49 ` [PATCH 5/5] clean/smudge: allow clean filters to process extremely large files Matt Cooper via GitGitGadget
2021-10-28 20:50 ` [PATCH v2 0/7] Allow clean/smudge filters to handle huge files in the LLP64 data model Johannes Schindelin via GitGitGadget
2021-10-28 20:50   ` [PATCH v2 1/7] test-genzeros: allow more than 2G zeros in Windows Carlo Marcelo Arenas Belón via GitGitGadget
2021-10-28 20:50   ` [PATCH v2 2/7] test-tool genzeros: generate large amounts of data more efficiently Johannes Schindelin via GitGitGadget
2021-10-28 22:55     ` Junio C Hamano
2021-10-28 20:50   ` [PATCH v2 3/7] t1051: introduce a smudge filter test for extremely large files Matt Cooper via GitGitGadget
2021-10-28 20:50   ` [PATCH v2 4/7] odb: teach read_blob_entry to use size_t Matt Cooper via GitGitGadget
2021-10-28 22:14     ` Carlo Arenas
2021-10-28 22:21       ` Johannes Schindelin
2021-10-28 20:50   ` [PATCH v2 5/7] git-compat-util: introduce more size_t helpers Johannes Schindelin via GitGitGadget
2021-10-28 23:05     ` Junio C Hamano
2021-10-28 20:50   ` [PATCH v2 6/7] odb: guard against data loss checking out a huge file Matt Cooper via GitGitGadget
2021-10-28 20:50   ` [PATCH v2 7/7] clean/smudge: allow clean filters to process extremely large files Matt Cooper via GitGitGadget
2021-10-28 22:32   ` [PATCH v2 0/7] Allow clean/smudge filters to handle huge files in the LLP64 data model brian m. carlson
2021-10-28 23:07     ` Junio C Hamano
2021-10-29 13:59   ` [PATCH v3 0/8] " Johannes Schindelin via GitGitGadget
2021-10-29 13:59     ` [PATCH v3 1/8] test-genzeros: allow more than 2G zeros in Windows Carlo Marcelo Arenas Belón via GitGitGadget
2021-10-29 13:59     ` [PATCH v3 2/8] test-tool genzeros: generate large amounts of data more efficiently Johannes Schindelin via GitGitGadget
2021-10-29 22:50       ` Junio C Hamano
2021-10-29 13:59     ` [PATCH v3 3/8] test-lib: add prerequisite for 64-bit platforms Carlo Marcelo Arenas Belón via GitGitGadget
2021-10-29 22:52       ` Junio C Hamano
2021-11-02 14:35         ` Johannes Schindelin
2021-10-29 13:59     ` [PATCH v3 4/8] t1051: introduce a smudge filter test for extremely large files Matt Cooper via GitGitGadget
2021-10-29 23:00       ` Junio C Hamano
2021-10-29 23:21         ` Junio C Hamano
2021-11-02 14:56           ` Johannes Schindelin
2021-11-02 14:57         ` Johannes Schindelin
2021-10-29 13:59     ` [PATCH v3 5/8] odb: teach read_blob_entry to use size_t Matt Cooper via GitGitGadget
2021-10-29 23:17       ` Junio C Hamano
2021-11-02 15:10         ` Johannes Schindelin
2021-10-29 13:59     ` [PATCH v3 6/8] git-compat-util: introduce more size_t helpers Johannes Schindelin via GitGitGadget
2021-10-29 23:10       ` Junio C Hamano
2021-10-29 13:59     ` [PATCH v3 7/8] odb: guard against data loss checking out a huge file Matt Cooper via GitGitGadget
2021-10-29 23:13       ` Junio C Hamano
2021-10-29 13:59     ` [PATCH v3 8/8] clean/smudge: allow clean filters to process extremely large files Matt Cooper via GitGitGadget
2021-10-29 23:17       ` Junio C Hamano
2021-11-02 14:59         ` Johannes Schindelin
2021-10-29 18:34     ` [PATCH v3 0/8] Allow clean/smudge filters to handle huge files in the LLP64 data model Junio C Hamano
     [not found]       ` <nycvar.QRO.7.76.6.2110292239170.56@tvgsbejvaqbjf.bet>
2021-10-29 21:12         ` Johannes Schindelin
2021-10-29 23:25           ` Junio C Hamano
2021-10-30 15:16           ` Philip Oakley
2021-10-30 17:35             ` Torsten Bögershausen
2021-10-30 19:29               ` Philip Oakley
2021-11-02 14:41       ` Johannes Schindelin
2021-11-02 15:46     ` Johannes Schindelin via GitGitGadget [this message]
2021-11-02 15:46       ` [PATCH v4 1/8] test-genzeros: allow more than 2G zeros in Windows Carlo Marcelo Arenas Belón via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 2/8] test-tool genzeros: generate large amounts of data more efficiently Johannes Schindelin via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 3/8] test-lib: add prerequisite for 64-bit platforms Carlo Marcelo Arenas Belón via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 4/8] t1051: introduce a smudge filter test for extremely large files Matt Cooper via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 5/8] odb: teach read_blob_entry to use size_t Matt Cooper via GitGitGadget
2021-11-02 20:40         ` Torsten Bögershausen
2021-11-04  0:09           ` Johannes Schindelin
2021-11-04 12:24             ` Philip Oakley
2021-11-02 15:46       ` [PATCH v4 6/8] git-compat-util: introduce more size_t helpers Johannes Schindelin via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 7/8] odb: guard against data loss checking out a huge file Matt Cooper via GitGitGadget
2021-11-02 15:46       ` [PATCH v4 8/8] clean/smudge: allow clean filters to process extremely large files Matt Cooper via GitGitGadget
2021-11-02 20:47         ` Torsten Bögershausen
2021-11-04  0:11           ` Johannes Schindelin
2021-11-04  8:33             ` Torsten Bögershausen
2021-11-04 17:26         ` Junio C Hamano
2021-11-02 21:46       ` [PATCH v4 0/8] Allow clean/smudge filters to handle huge files in the LLP64 data model Torsten Bögershausen
2021-11-03  6:31         ` Johannes Sixt
2021-10-28 20:56 ` [PATCH 0/3] " Carlo Marcelo Arenas Belón
2021-10-28 20:56   ` [PATCH 1/3] test-lib: add prerequisite for 64-bit platforms Carlo Marcelo Arenas Belón
2021-10-28 21:45     ` Johannes Schindelin
2021-10-28 22:09       ` Carlo Arenas
2021-10-28 22:38         ` Junio C Hamano
2021-11-02 15:20           ` Johannes Schindelin
2021-10-28 20:56   ` [PATCH 2/3] fixup! t1051: introduce a smudge filter test for extremely large files Carlo Marcelo Arenas Belón
2021-10-28 20:56   ` [PATCH 3/3] fixup! clean/smudge: allow clean filters to process " Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1068.v4.git.1635867971.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=philipoakley@iee.email \
    --cc=sandals@crustytoothpaste.net \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).