Docker build image locales error #1362
Conversation
| RUN yarn run build:prod | ||
| RUN rm -rf /grist/static/locales |
There was a problem hiding this comment.
Hmm, I may be wrong, but I fear it would not be as simple as this.
The fact is that yarn run build:prod invokes a tool to sanitize the localized strings. If we place them just to make yarn run build:prod work, but remove them right after, I think there would be no point for introducing this util: #1354
| RUN yarn run build:prod | ||
| RUN rm -rf /grist/static/locales |
There was a problem hiding this comment.
Hmm. The error seems to come from a new sanitization step applied to the locales, as @fflorent noted here #1361 (comment). If the sanitization step has an effect, does that effect get propagated to the final image? Reading this, I'm worried an unsanitized version of the locale files ends up in the final image.
Would like @berhalak and @georgegevoian's opinions, they should be back from breaks tomorrow and Wednesday respectively.
(Side note: I wonder if something that needs sanitization is found, the build should break rather than try to continue? Not sure if this new step is there to support some desired syntax or to avoid some undesired syntax. If the latter, failing loudly might be the right thing to do.)
There was a problem hiding this comment.
I added this line to avoid the 151 line error:
# Finalize static directory
RUN \
mv /grist/static-built/* /grist/static && \
rmdir /grist/static-built
It has error:
=> ERROR [stage-5 23/25] RUN mv /grist/static-built/* /grist/static && rmdir /grist/static-built
------
> [stage-5 23/25] RUN mv /grist/static-built/* /grist/static && rmdir /grist/static-built:
0.443 mv: inter-device move failed: '/grist/static-built/locales' to '/grist/static/locales'; unable to remove target: Directory not empty
I think there is a problem in the steps order: locales are not sanitized at the right time in docker image build.
There was a problem hiding this comment.
@tristanrobert that makes sense. @berhalak has tweaked the script to just fail if the translations are detected to contain dodgy material, and included the Dockerfile update (#1367). Thanks for your help with this!
There was a problem hiding this comment.
Code is finally included in #1367 so it could be closed.
|
Fixed elsewhere #1362 (comment), thanks @tristanrobert |
Fixes #1361
Context
Proposed solution
Add
/static/localesin container before build and remove it after.Related issues
Has this been tested?
Screenshots / Screencasts