commands: Improve chtimes permission hint#14938
Conversation
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. |
| func TestWrapStaticSyncError(t *testing.T) { | ||
| c := qt.New(t) | ||
|
|
||
| err := wrapStaticSyncError(&os.PathError{Op: "chtimes", Path: "public", Err: syscall.EPERM}) |
There was a problem hiding this comment.
In my head there are 2 types of tests:
- Unit tests (tests e.g. a function in isolation)
- Integration tests (tests more or less the entire chain with as a real a setup as practical)
This test falls between these two and I question its value. A test for a bug fix needs to 1. Demonstrate that we have fixed the bug and 2. Prevent future regressions of that same bug. The error created here is a fake one, which means that I don't know if it matches reality.
I'm assuming that testing the real scenario (e.g. with a testscript) would not be practical, so I suggest just removing the test. I have no problem seeing what the wrapper func does, and I assume it will work, and if not if will not make it worse.
There was a problem hiding this comment.
Removed the synthetic test as suggested in 110d69d45.
I re-ran PATH="/Users/xndvaz/go/bin:$PATH" ./check.sh ./commands/... and PATH="/Users/xndvaz/go/bin:$PATH" ./check.sh locally before pushing. The PR checks are re-running now.
|
@xndvaz have you manually tested this patch and can confirm that this is works as described? |
This does what it says it should do, but I don't think it improves the user experience in relation to the conditions described in #7302. $ sudo sh -c "mkdir public && chmod a+w public"
$ hugo
Start building sites …
hugo v0.163.0-DEV-889ce2c161636ff9e04db6f507fae5bbb273a92a+withdeploy linux/amd64 BuildDate=2026-06-01T18:48:36Z VendorInfo=jmooring
Total in 11 ms
ERROR error copying static files: chtimes /home/jmooring/code/hugo-testing/public: operation not permitted; if the publishDir is writable but not owned by the Hugo process, try --noTimes or set noTimes = true
$ hugo --noTimes # same error when combined with --noChmod
Start building sites …
hugo v0.163.0-DEV-889ce2c161636ff9e04db6f507fae5bbb273a92a+withdeploy linux/amd64 BuildDate=2026-06-01T18:48:36Z VendorInfo=jmooring
Total in 10 ms
ERROR error copying static files: stat /home/jmooring/code/hugo-testing/public/favicon.ico: permission deniedSo you fix one error, and then encounter another. It seems to me we should just state something like "you have insufficient permissions to write files to the publishdDir". |
3 participants
Fixes #7302
Summary
chtimeshugo serverstatic sync loggingchtimespermission case and non-matching errorsRoot cause
When the publishDir is writable but not owned by the Hugo process, the OS can reject
chtimesduring static sync. Hugo surfaced the raw error, but did not point users to the existing--noTimes/noTimes = trueworkaround.Validation
PATH="/Users/xndvaz/go/bin:$PATH" ./check.sh ./commands/...PATH="/Users/xndvaz/go/bin:$PATH" ./check.shAI assistance
This PR was written with Codex assistance. I reviewed the generated changes and validation results.