Skip to content

feat: Add make targets to build snap from worktree - #292

Draft
MaikRe wants to merge 2 commits into
canonical:masterfrom
MaikRe:snap-worktree
Draft

MaikRe wants to merge 2 commits into
canonical:masterfrom
MaikRe:snap-worktree

Conversation

@MaikRe

@MaikRe MaikRe commented May 29, 2026

Copy link
Copy Markdown
Contributor

Adds two new make targets and additional supporting functionality to make the UI and docs build works

make snap-work-tree functions essentially identical to make snap-tree expect that it cannot fetch the UI commit hash so it must be build locally.
make snap-work-tree-sync functions essentially identical to make snap-tree-sync.

Testing procedure:

cd /path/to/your/maas/repo
git worktree add ../new-worktree -b branch_name maik/snap-worktree # Specifically checkout this branch as obviously need the make targets
cd ../new-worktree
# To test from a truly clean environment
make snap-clean
make snap-tree-clean

make snap-work-tree

The work tree can then be mounted into an LXD container etc and updated with make snap-work-tree-sync

@MaikRe
MaikRe requested a review from a team May 29, 2026 22:06
@maas-lander

maas-lander commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Check where you would like a Mattermost message to be sent to when CI completes and this PR is merged

  • Direct message
  • ~maas

@MaikRe MaikRe changed the title Add make targets to build snap from worktree feat: Add make targets to build snap from worktree May 29, 2026
@MaikRe
MaikRe marked this pull request as draft May 29, 2026 22:07
@MaikRe
MaikRe marked this pull request as ready for review June 4, 2026 17:55
Comment thread docs/conf.py
):

def _has_usable_git_worktree() -> bool:
repo_root = Path(__file__).resolve().parents[1]

@AloizioMacedo AloizioMacedo Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You don't need to go to the repo root, git rev-parse --is-inside-work-tree should figure things out correctly (this would only be a potential problem in something like the maasui dir, which is a submodule).

So you can just use cwd=Path(__file__).resolve().parents[0]. Since we are already cleaning up the awkward check, this also gets a little more resilience since we don't need to assume we are inside another dir. Theoretically there should be no problem in building the docs with the docs folder as root, in principle (e.g., if I mv everything of the docs folder inside the root of a docker container things could just work) and the current version would panic with a IndexError in that case. (Practically, we still use references to the package due to the API/CLI reference, but this will most likely change in the future, and there is no need to make extraneous assumptions where not necessary)

Comment thread Makefile
Comment on lines +473 to +482
snap-work-tree:
@if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then \
git submodule update --init --recursive src/maasui/src; \
else \
echo "Skipping submodule update: git worktree metadata unavailable"; \
fi
$(MAKE) --no-print-directory snap-tree
.PHONY: snap-work-tree

snap-work-tree-sync:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the changes proposed in place, those could be actually replacements of snap-tree and snap-tree-sync, right? They would work by having a standard git clone'd repo as well.

Maybe you are just trying to prevent overwriting the usual way due to potential unforeseen issues. In that case, maybe opening a new PR later with just the replacement itself? The point being that this is a kind of duplication.

@MaikRe MaikRe Jun 22, 2026

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.

Maybe you are just trying to prevent overwriting the usual way due to potential unforeseen issues. In that case, maybe opening a new PR later with just the replacement itself? The point being that this is a kind of duplication.

Yes that is my primary issue.
But for now I think I will table this until the monorepo #259 discussion is done as that might simplify some things as well.

@MaikRe
MaikRe marked this pull request as draft June 22, 2026 22:52
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.

3 participants