Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CodeStyle][py36] Update dockerfile and some script #49558

Merged
merged 14 commits into from
Feb 6, 2023

Conversation

gsq7474741
Copy link
Contributor

@gsq7474741 gsq7474741 commented Jan 4, 2023

@paddle-bot
Copy link

paddle-bot bot commented Jan 4, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@gsq7474741
Copy link
Contributor Author

@luotao1 luotao1 self-assigned this Jan 5, 2023
@luotao1
Copy link
Contributor

luotao1 commented Jan 10, 2023

maccheck_py35 会被运行,去掉后ci报错

@tianshuo78520a 后续帮忙处理下。

@gsq7474741 请解决下 CodeStyle 流水线的格式问题

@@ -64,7 +64,7 @@ function do_cpython_build {
local prefix="/opt/_internal/cpython-${py_ver}${dir_suffix}"
mkdir -p ${prefix}/lib
# -Wformat added for https://bugs.python.org/issue17547 on Python 2.6

# TODO(gsq7474741): 这里要求的python版本是否需要更新?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsq7474741 因为79行开始有对py37的处理,所以68-75行可以直接删掉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些操作 #48490 已经做了,这个 PR 先不动这几个文件吧,我看那个 PR 已经清理地很彻底了~

以下几个文件这个 PR 可以先不动:

  • tools/dockerfile/build_scripts/build.sh
  • tools/dockerfile/build_scripts/build_utils.sh
  • tools/dockerfile/ci_dockerfile.sh
  • tools/dockerfile/Dockerfile.centos

tools/dockerfile/build_scripts/build_utils.sh Outdated Show resolved Hide resolved
tools/dockerfile/build_scripts/build_utils.sh Outdated Show resolved Hide resolved
tools/dockerfile/ci_dockerfile.sh Outdated Show resolved Hide resolved
@luotao1
Copy link
Contributor

luotao1 commented Jan 11, 2023

maccheck_py35 改名 为maccheck_py3:

# Conflicts:
#	paddle/scripts/paddle_build.sh
#	tools/dockerfile/build_scripts/build.sh
#	tools/dockerfile/build_scripts/build_utils.sh
run_mac_test ${PYTHON_ABI:-""} ${PROC_RUN:-1}
;;
maccheck_py3)
cmake_gen_and_build_mac ${PYTHON_ABI:-""}
Copy link
Contributor

@luotao1 luotao1 Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4111行和4115行有两个maccheck_py3(4111行应该是maccheck),重复了。merge最新的develop后,这块不用改了。

@@ -77,19 +77,7 @@ function cmake_base() {
SYSTEM=`uname -s`
if [ "$SYSTEM" == "Darwin" ]; then
echo "Using python abi: $1"
if [ "$1" == "cp36-cp36m" ] || [ "$1" == "" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里[ "$1" == "" ]的case是不是漏掉了?py37判断时加上这个case吧

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XieYunshen 再review下 conda_build.py

@gsq7474741
Copy link
Contributor Author

已更新

Copy link
Contributor

@pangyoki pangyoki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@XieYunshen XieYunshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for conda_build.py

@luotao1 luotao1 merged commit 1a4edcb into PaddlePaddle:develop Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants