Copilot commented on code in PR #13597:
URL: https://github.com/apache/cloudstack/pull/13597#discussion_r3577455588
##########
tools/docker/Dockerfile:
##########
@@ -61,12 +62,10 @@ RUN find /var/lib/mysql -type f -exec touch {} \; && \
mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH
caching_sha2_password by ''" --connect-expired-password; \
mvn -Pdeveloper -pl developer -Ddeploydb; \
mvn -Pdeveloper -pl developer -Ddeploydb-simulator; \
- MARVIN_FILE=`find /root/tools/marvin/dist/ -name "Marvin*.tar.gz"`; \
- rm -rf /usr/bin/x86_64-linux-gnu-gcc && \
- ln -s /usr/bin/gcc-10 /usr/bin/x86_64-linux-gnu-gcc; \
- pip3 install $MARVIN_FILE
+ MARVIN_FILE=`find /root/tools/marvin/dist/ -name "[mM]arvin*.tar.gz"`; \
+ pip3 install $MARVIN_FILE --break-system-packages
Review Comment:
The MARVIN_FILE variable is unquoted, and `find` can return multiple matches
(or paths with whitespace), which can cause word-splitting and unintended
arguments to `pip3 install`. Prefer using `$(...)`, limiting to a single match
(e.g., `-print -quit`), and quoting the variable when passing it to pip.
##########
tools/docker/Dockerfile:
##########
@@ -61,12 +62,10 @@ RUN find /var/lib/mysql -type f -exec touch {} \; && \
mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH
caching_sha2_password by ''" --connect-expired-password; \
mvn -Pdeveloper -pl developer -Ddeploydb; \
mvn -Pdeveloper -pl developer -Ddeploydb-simulator; \
- MARVIN_FILE=`find /root/tools/marvin/dist/ -name "Marvin*.tar.gz"`; \
- rm -rf /usr/bin/x86_64-linux-gnu-gcc && \
- ln -s /usr/bin/gcc-10 /usr/bin/x86_64-linux-gnu-gcc; \
- pip3 install $MARVIN_FILE
+ MARVIN_FILE=`find /root/tools/marvin/dist/ -name "[mM]arvin*.tar.gz"`; \
+ pip3 install $MARVIN_FILE --break-system-packages
Review Comment:
Using `--break-system-packages` makes the image more brittle by installing
into the system Python in a way that can conflict with OS-managed packages. A
more robust approach is to create a dedicated virtual environment for Marvin
(or install via distro packages where available) and run the simulator using
that environment.
##########
tools/docker/supervisord.conf:
##########
@@ -16,7 +17,8 @@ redirect_stderr=true
user=root
[program:cloudstack-ui]
-command=/bin/bash -c "npm run serve"
+environment=NODE_OPTIONS="--openssl-legacy-provider"
+command=/bin/bash -c "NODE_OPTIONS=--openssl-legacy-provider npm run serve"
Review Comment:
NODE_OPTIONS is set twice: once via Supervisor `environment` and again
inline in the command. This is redundant and makes it unclear which value
should be authoritative. Prefer keeping it in one place (typically the
`environment` line) and simplifying the `command` accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
