Re: [Buildroot] [PATCH 5/5] support/download/git: handle git attributes

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Yann,

On Wed, 13 Sep 2023 00:15:52 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Files in a git repository can be given attributes, like the usual eol
> that can convert to-from crlf, cr, lf when checking-out of committing a
> file.
> 
> There are also two attributes that are meant to be used when generating
> an archive (with git archive): export-subst, and export-ignore, that
> respectively substitutes format placeholders in a file, and excludes a
> file from the archive.
> 
> Some package (e.g. pcm-tools, luajit) use the export-subst attribute
> to generate versioning information. luajit, specifically, uses the UNIX
> timestamp of the commit as the patch-level for its semantic versioning.
> 
> Extend the git backend to handle the export-subst attribute. There is
> no git tool (that we could find) that does that automatically, except
> git-archive, which we can't use. So, we do it manually with a bit of
> trickery where we do use awk to:
> 
>   - identify all files that have the export-subst attribute set,
> 
>   - for each such file, iterate over all the placeholders, and
>     replace them with the expanded format.
> 
> When doing the replacement, we decided to force abbreviating short
> hashes to 40 chars, which is the length of a full sha1, rather than
> actually abbreviating them:
> 
>   - letting git decide of the length is not reproducible over time:
>     as new commits are added, the short length will increase to avoid
>     collisions; a newer git version may decide of a different heuristic
>     to shorten hashes; a user may have a local setting with an arbitrary
>     length;
> 
>   - deciding on our side of an arbitrary value would not be
>     viable long term either, as it might be too large to be minimum, or
>     too short to avoid collisions.
> 
> The only reproducible solution is to use unabbreviated hashes.
> 
> Handling git-attributes also implies that the format of the generated
> archives has changed, since we now expand placeholders, so we bump our
> git format version.
> 
> Of all our git-downloaded packages, 5 are affected:
> 
>   - pcm-tools, which was known, and the one that triggered this commit;
>     since we now expand placeholders, we can drop the post-extract hook;
>     switching to a full hash in replacements also changes the hash of
>     the generated archive;
> 
>   - qt5knx, qt5location, qt5mqtt, and qt5opcua: the file .tag at the
>     repository root, contains only the full hash placeholder; that file
>     is not used at all during the build (AFAICS);
> 
> Finally, we add a run-time test to validate this new feature.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Woody Douglass <wdouglass@carnegierobotics.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Francois Perrad <fperrad@gmail.com>

I am not totally convinced by this. I recognize the massive efforts
that have been into these complex awk scripts, but there are a few
reasons why I dislike it:

(1) It's complex/non-obvious

(2) It fixes a problem in only one single package, which is already
    fixed in a package-specific way, and we don't have visibility on
    many more packages adopting this.

(3) It bumps the Git tarball version, which is going to cause breakage
    to *all* downstream users of Buildroot which have custom packages
    retrieved from Git (and there are probably thousands of these in
    the numerous companies using Buildroot). So causing so much hassle
    to our users for the gain of fixing one package in Buildroot is IMO
    not a good trade-off.

Of course, I can be convinced that I'm wrong, but I'm not too convinced
at the moment. And of course (again), Peter and Arnout are definitely
welcome to disagree and overrule me :)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot