Skip to content

Conversation

lemeurherveCB
Copy link
Contributor

@lemeurherveCB lemeurherveCB commented Jun 30, 2025

This PR installs git-lfs from its repository instead of relying on Linux distribution packages, to ensure we're getting the same version installed accross all images.

Ref:

I'll open another PR for updatecli manifest.

Testing done

make build test

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

@lemeurherveCB lemeurherveCB requested a review from a team as a code owner June 30, 2025 13:53
@lemeurherveCB lemeurherveCB marked this pull request as draft June 30, 2025 17:08
@lemeurherve lemeurherve marked this pull request as ready for review October 9, 2025 17:20
@lemeurherve lemeurherve linked an issue Oct 9, 2025 that may be closed by this pull request
&& ln -sf /usr/share/jenkins/agent.jar /usr/share/jenkins/slave.jar

ARG GIT_LFS_VERSION=3.7.0
RUN arch=$(uname -m | sed -e 's/x86_64/amd64/g' -e 's/aarch64/arm64/g' -e 's/armv7l/arm/g') \
Copy link
Member

@lemeurherve lemeurherve Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this third sed replacement pattern to Debian images which include Linux/arm/v7 architecture

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit question: wouldn't it work with dpkg --print-architecture? (For arm64 it works, but I'm not sure for arm/v7)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested. What would be the benefit of using dpkg instead of uname in that case?

Copy link
Contributor

@dduportal dduportal Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dpkg --print-architecture (I'm not 100% sure of the flag, worth checking the doc.) is usually called when you expect a "Golang/Docker" CPU architecture string such as amd64 or arm64 instead of the Kernel designations from uname.

It avoids doing tedious text search and replace techniques (whatever techniques: shell search/replace, sed, awk, perl, etc.).

Copy link
Member

@lemeurherve lemeurherve Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dduportal after testing, as dpkg is not available in Alpine nor in UBI9 images, and as dpkg --print-architecture returns armhf for Linux/arm/v7 and thus still requires a sed into arm in that case, I propose to keep uname -m in all images for uniformity.

WDYT?

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the test checking we can, at least, instantiate the binary (Golang binary), the Alpine and Ubi implementations should be good (even if we have to be aware that we might break them since the static binary could rely on other libraries or packages) . But LGTM!

@dduportal dduportal merged commit 20ad00f into jenkinsci:master Oct 17, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't rely on linux distribution's package for git-lfs

3 participants