Update manylinux build scripts #10467
Conversation
| RUN export SQLITE_AUTOCONF_ROOT=sqlite-autoconf-3360000 && \ | ||
| export SQLITE_AUTOCONF_HASH=bd90c3eb96bee996206b83be7065c9ce19aef38c3f4fb53073ada0d0b69bbce3 && \ | ||
| export SQLITE_AUTOCONF_DOWNLOAD_URL=https://www.sqlite.org/2021 && \ | ||
| RUN export SQLITE_AUTOCONF_ROOT=sqlite-autoconf-3370200 && \ |
There was a problem hiding this comment.
can we put these variable definitions into a common place so that we don't need to update different files when the version changes?
e.g., RUN source export_sqlite_vars.sh && manylinux-entrypoint /build_scripts/build-sqlite3.sh
export_sqlite_vars.sh:
export SQLITE_AUTOCONF_ROOT=sqlite-autoconf-3370200
export SQLITE_AUTOCONF_HASH=4089a8d9b467537b3f246f217b84cd76e00b1d1a971fe5aca1e30e230e46b2d8 && \
export SQLITE_AUTOCONF_DOWNLOAD_URL=https://www.sqlite.org/2022
we'd also need to update the cgmanifest file generation
There was a problem hiding this comment.
I just tried it. Not very good. Currently I just copy and paste the code from manylinux repo to ours. If there are too many Dockerfiles to modify, I can make a script. I already put makers in the docker files.
#Build manylinux2014 docker image begin
...
#Build manylinux2014 docker image end
If I extract the export commands to a separated file, I think it will need more human work to maintain.
There was a problem hiding this comment.
Can docker file include another docker file?
There was a problem hiding this comment.
Could you elaborate on how it's more work to maintain?
I counted 11 instances of SQLITE_AUTOCONF_ROOT in the changes (different Dockerfiles). Changing it in one place seems less error-prone and easier to review.
Updating them with a script is a good idea too.
|
It would be nice to have a one-two lines descritpion such as advancing commits for this and that due to so it sets a frame of mind. |
|
This issue has been automatically marked as stale due to inactivity and will be closed in 7 days if no further activity occurs. If further support is needed, please provide an update and/or more details. |
stale
bot
added
the
stale
|
Will close it. It's better to keep them up to date, which means do such things regularly. But it is not required. |
Description:
Update manylinux build scripts
Motivation and Context
In preparation for #9782. It is just to keep the dependencies of our build environment up to date. We don't have to do it unless there are known security vulnerabilities in the tools we use(like autoconf/automake/patchelf), however, it is also not harmful if we do so. Our default CPU build always use the latest. So, changes like this just impact the other packages like CUDA/TRT/... .
I plan to add musl support and get #8754 merged after this one is done.