Skip to content

maint(linux): create temporary worktree for packaging#14598

Merged
ermshiperete merged 6 commits intomasterfrom
maint/linux/packaging
Sep 1, 2025
Merged

maint(linux): create temporary worktree for packaging#14598
ermshiperete merged 6 commits intomasterfrom
maint/linux/packaging

Conversation

@ermshiperete
Copy link
Copy Markdown
Contributor

This change creates a temporary worktree for creating Debian packages. This will allow us to work from a clean state of the source repo. Also exclude any generated patch directories (.pc).

Build-bot: skip
Test-bot: skip

This change creates a temporary worktree for creating Debian packages.
This will allow us to work from a clean state of the source repo. Also
exclude any generated patch directories (`.pc`).

Build-bot: skip
Test-bot: skip
@github-project-automation github-project-automation bot moved this to Todo in Keyman Aug 25, 2025
@github-actions github-actions bot added linux/ maint Maintenance work -- continuous integration, build scripts, infrastructure labels Aug 25, 2025
@keymanapp-test-bot
Copy link
Copy Markdown

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S10 milestone Aug 25, 2025
ermshiperete added a commit that referenced this pull request Aug 25, 2025
This change creates a temporary worktree for creating Debian packages.
This will allow us to work from a clean state of the source repo. Also
exclude any generated patch directories (`.pc`).

Cherry-pick-of: #14598
Build-bot: skip
Test-bot: skip
Copy the generated files before removing the worktree so that they
are still available for reference.
cp -r linux/debianpackage "${KEYMAN_ROOT}/linux"
fi

cd "${PREV_DIR:-}" || true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a little worrying, why ignore failure? If it fails and we have a PWD inside the WORKTREE_DIR, will this cause a flow-on failure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +130 to +131
git worktree remove -f "${WORKTREE_DIR}" || true
git branch -f -D "${WORKTREE_BRANCH}" || true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are the circumstances of failure here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +122 to +124
if [[ -d "linux/debianpackage" ]]; then
cp -r linux/debianpackage "${KEYMAN_ROOT}/linux"
fi
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like this could start in an arbitrary folder if a trap is thrown

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor Author

@ermshiperete ermshiperete left a comment

Choose a reason for hiding this comment

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

Replied to comments

Comment on lines +122 to +124
if [[ -d "linux/debianpackage" ]]; then
cp -r linux/debianpackage "${KEYMAN_ROOT}/linux"
fi
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

cp -r linux/debianpackage "${KEYMAN_ROOT}/linux"
fi

cd "${PREV_DIR:-}" || true
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +130 to +131
git worktree remove -f "${WORKTREE_DIR}" || true
git branch -f -D "${WORKTREE_BRANCH}" || true
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@ermshiperete ermshiperete requested a review from mcdurdin August 25, 2025 15:50
WORKTREE_BRANCH="maint/linux/tmp-packaging-${DEPLOY_BRANCH#origin/}"
PREV_DIR="$PWD"

WORKTREE_DIR="$(git worktree list | grep "${WORKTREE_BRANCH}" | cut -d' ' -f1)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should be using --porcelain where available

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +153 to +155
if [[ -n "${WORKTREE_DIR}" ]] && [[ -d "${WORKTREE_DIR}" ]]; then
builder_heading "Reusing existing worktree at ${WORKTREE_DIR}"
git checkout -B "${WORKTREE_BRANCH}" "${DEPLOY_BRANCH#origin/}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When would the worktree already exist?

Are we in the right folder to do this checkout? Shouldn't this be done from the worktree folder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had it happen that for whatever reason the worktree directory didn't get deleted. Probably not happening if the script works right, but...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess in that situation I would be more comfortable with deleting the folder and cleanly recreating the worktree?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

WORKTREE_DIR="$(mktemp -d)"
builder_heading "Creating temporary worktree at ${WORKTREE_DIR}"
if git branch | grep -q "${WORKTREE_BRANCH}"; then
git branch -f -D "${WORKTREE_BRANCH}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
git branch -f -D "${WORKTREE_BRANCH}"
git branch -D "${WORKTREE_BRANCH}"

-D is equivalent to --delete --force

if git branch | grep -q "${WORKTREE_BRANCH}"; then
git branch -f -D "${WORKTREE_BRANCH}"
fi
git worktree add -f -b "${WORKTREE_BRANCH}" "${WORKTREE_DIR}" "${DEPLOY_BRANCH#origin/}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would hope we wouldn't need -f here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

# target branch (stable/beta). Checkout stable/beta branch so that
# scripts/debian.sh picks up correct version

WORKTREE_BRANCH="maint/linux/tmp-packaging-${DEPLOY_BRANCH#origin/}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

${DEPLOY_BRANCH#origin/} seems to be used repeatedly. May be worth putting into a variable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@ermshiperete ermshiperete requested a review from mcdurdin August 29, 2025 10:34
@darcywong00 darcywong00 modified the milestones: A19S10, A19S11 Aug 29, 2025
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

LGTM!

@ermshiperete ermshiperete merged commit a3ec6be into master Sep 1, 2025
6 checks passed
@ermshiperete ermshiperete deleted the maint/linux/packaging branch September 1, 2025 13:11
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Sep 1, 2025
@keyman-server
Copy link
Copy Markdown
Collaborator

Changes in this pull request will be available for download in Keyman version 19.0.110-alpha

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

Labels

linux/ maint Maintenance work -- continuous integration, build scripts, infrastructure

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants