app: make build/.gitkeep postbuild step git-worktree friendly#1311
Conversation
The `postbuild` script for the web UI invokes `git restore build/.gitkeep` to bring back the placeholder file after react-scripts wipes the build directory. When the repository is checked out as a `git worktree`, the top-level `.git` is a file that points at a worktree directory on the host. The dev.Dockerfile build copies the working tree into the container with `COPY .`, but the path referenced by that `.git` pointer is not part of the build context, so any git invocation inside the container aborts with `fatal: not a git repository`, causing `yarn build` and the whole image build to fail. Fall back to writing the `.gitkeep` content directly when `git restore` is not usable. `git restore` is still preferred so that the byte-for-byte contents from the index are used when git is available; the fallback only kicks in when git cannot reach the worktree, which is exactly the case inside the dev image build.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a build failure occurring when the project is built inside a Docker container while the host environment uses git worktrees. By making the postbuild script tolerant of git failures, the build process can now successfully complete even when the git repository structure is not fully accessible within the build context. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the postbuild script in app/package.json to ensure the build/.gitkeep file is restored or recreated after a build. A review comment correctly identifies that the use of POSIX-specific shell syntax (2>/dev/null and printf) will cause failures on Windows environments and suggests a cross-platform Node.js one-liner as a replacement.
| "develop": "REACT_APP_USE_SAMPLE_DATA=true yarn start", | ||
| "build": "react-scripts build", | ||
| "postbuild": "git restore build/.gitkeep", | ||
| "postbuild": "git restore build/.gitkeep 2>/dev/null || printf '# Keep directory in git.\\n' > build/.gitkeep", |
There was a problem hiding this comment.
The updated postbuild script uses POSIX-specific syntax (2>/dev/null and printf) which is not compatible with Windows cmd.exe. This will cause the build to fail for developers on Windows. To maintain cross-platform compatibility, consider using a Node.js one-liner to handle the git restoration and fallback logic.
| "postbuild": "git restore build/.gitkeep 2>/dev/null || printf '# Keep directory in git.\\n' > build/.gitkeep", | |
| "postbuild": "node -e \"try { require('child_process').execSync('git restore build/.gitkeep', { stdio: 'ignore' }); } catch (e) { require('fs').writeFileSync('build/.gitkeep', '# Keep directory in git.\\n'); }\"", |
There was a problem hiding this comment.
I don't think this is needed as this is for Docker builds (i.e. linux).
ViktorT-11
left a comment
There was a problem hiding this comment.
Thanks, I think this change looks good!
It'd be valuable if @jamaljsr could take a quick look at this PR as well as it's docker related.
Additionally @ZZiigguurraatt, can you test this to see that it resolves your issue?
| "develop": "REACT_APP_USE_SAMPLE_DATA=true yarn start", | ||
| "build": "react-scripts build", | ||
| "postbuild": "git restore build/.gitkeep", | ||
| "postbuild": "git restore build/.gitkeep 2>/dev/null || printf '# Keep directory in git.\\n' > build/.gitkeep", |
There was a problem hiding this comment.
I don't think this is needed as this is for Docker builds (i.e. linux).
jamaljsr
left a comment
There was a problem hiding this comment.
tACK LGTM 👌
Confirmed the broken docker build on a worktree which is resolved with this fix. Thanks for the contribution.
Fixes #1022.
Problem
When the working copy of
lightning-terminalis a git worktree, the top-level.gitis a file likegitdir: ../.bare/worktrees/<name>rather than a real.git/directory. Building withdev.Dockerfilethen fails:This happens because
dev.Dockerfilecopies the working tree into the container withCOPY ., but the path that the.gitfile points at lives outside the build context, so any git invocation inside the container errors out. The first git call inside the build comes from thepostbuildscript inapp/package.json:That script aborts,
yarn buildfails, and the image build dies.Fix
Make the
postbuildstep tolerant of environments where git is not usable.git restoreis still preferred — it preserves the exact byte contents from the index, including any line-ending normalisation the originalapp: use git restore to re-create .gitkeep filecommit was guarding against — and the fallback only kicks in when git cannot reach the worktree (which is exactly the case inside the dev image build):Verification
yarn --cwd app buildstill restoresbuild/.gitkeepfrom the index viagit restore(the first branch of the||).dev.Dockerfilesituation), the second branch runs and writes the same content the file has atHEAD, sogit diffis still clean once the file is back on the host.