Refine API.spec mechanism (#20574)

* refine API.spec, test=develop * add nn.py to test,test=develop * fix by removing new.spec, test=develop, test=document_fix * refine scripts when PYTHON_ABI is empty, follow partial comments, add extra cpp api doc, test=develop, test=document_fix * remove extra doc changes, remove API.spec, test=develop
6 years ago · 40c258a77b
parent bc4af386f4
commit 40c258a77b
5 changed files with 103 additions and 1218 deletions
--- a/.gitignore
+++ b/.gitignore
@ -1,10 +1,7 @@
-paddle/operators/check_t.save
-paddle/operators/check_tensor.ls
-paddle/operators/tensor.save
-python/paddle/v2/fluid/tests/book/image_classification_resnet.inference.model/
-python/paddle/v2/fluid/tests/book/image_classification_vgg.inference.model/
-python/paddle/v2/fluid/tests/book/label_semantic_roles.inference.model/
 paddle/fluid/operators/distributed/send_recv.proto
+paddle/fluid/API.spec
+paddle/fluid/API_DEV.spec
+paddle/fluid/API_PR.spec
 *.DS_Store
 *.vs
 build/
--- a/paddle/fluid/API.spec
+++ b/paddle/fluid/API.spec
--- a/paddle/scripts/paddle_build.sh
+++ b/paddle/scripts/paddle_build.sh
@ -20,6 +20,10 @@

 set -ex

+if [ -z ${BRANCH} ]; then
+    BRANCH="develop"
+fi
+
 function print_usage() {
    echo -e "\n${RED}Usage${NONE}:
    ${BOLD}${SCRIPT_NAME}${NONE} [OPTION]"
@ -36,7 +40,6 @@ function print_usage() {
    ${BLUE}fluid_inference_lib${NONE}: deploy fluid inference library
    ${BLUE}check_style${NONE}: run code style check
    ${BLUE}cicheck${NONE}: run CI tasks
-    ${BLUE}assert_api_not_changed${NONE}: check api compability
    "
 }

@ -50,6 +53,8 @@ function init() {
    if [ -z "${SCRIPT_NAME}" ]; then
        SCRIPT_NAME=$0
    fi
+
+    ENABLE_MAKE_CLEAN=${ENABLE_MAKE_CLEAN:-ON}
 }

 function cmake_base() {
@ -291,7 +296,11 @@ function build_base() {
    if [ "$1" != "" ]; then
      parallel_number=$1
    fi
-    make clean
+
+    if [[ "$ENABLE_MAKE_CLEAN" != "OFF" ]]; then
+        make clean
+    fi
+
    make -j ${parallel_number}
    make install -j ${parallel_number}
 }
@ -304,7 +313,7 @@ function build() {
    Building in /paddle/build ...
    ============================================
 EOF
-    build_base $1
+    build_base $@
 }

 function build_mac() {
@ -315,7 +324,9 @@ function build_mac() {
    Building in /paddle/build ...
    ============================================
 EOF
-    make clean
+    if [[ "$ENABLE_MAKE_CLEAN" != "OFF" ]]; then
+        make clean
+    fi
    make -j 8
    make install -j 8
 }
@ -442,31 +453,69 @@ EOF
    fi
 }

-function assert_api_not_changed() {
+function fetch_upstream_develop_if_not_exist() {
+    UPSTREAM_URL='https://github.com/PaddlePaddle/Paddle'
+    origin_upstream_url=`git remote -v | awk '{print $1, $2}' | uniq | grep upstream | awk '{print $2}'` 
+    if [ "$origin_upstream_url" == "" ]; then
+        git remote add upstream $UPSTREAM_URL.git
+    elif [ "$origin_upstream_url" != "$UPSTREAM_URL" ] \
+            && [ "$origin_upstream_url" != "$UPSTREAM_URL.git" ]; then
+        git remote remove upstream
+        git remote add upstream $UPSTREAM_URL.git
+    fi
+    
+    if [ ! -e "$PADDLE_ROOT/.git/refs/remotes/upstream/$BRANCH" ]; then 
+        git fetch upstream # develop is not fetched
+    fi
+}  
+
+function generate_upstream_develop_api_spec() {
+    fetch_upstream_develop_if_not_exist
+    cur_branch=`git branch | grep \* | cut -d ' ' -f2` 
+    git checkout -b develop_base_pr upstream/$BRANCH
+    cmake_gen $1
+    build $2
+    generate_api_spec "$1" "DEV"
+    git checkout $cur_branch
+    git branch -D develop_base_pr 
+    ENABLE_MAKE_CLEAN="OFF"
+}
+
+function generate_api_spec() {
+    spec_kind=$2
+    if [ "$spec_kind" != "PR" ] && [ "$spec_kind" != "DEV" ]; then
+        echo "Not supported $2"
+        exit 1
+    fi
+
    mkdir -p ${PADDLE_ROOT}/build/.check_api_workspace
    cd ${PADDLE_ROOT}/build/.check_api_workspace
-    virtualenv .env
-    source .env/bin/activate
+    virtualenv .${spec_kind}_env
+    source .${spec_kind}_env/bin/activate
    pip install ${PADDLE_ROOT}/build/python/dist/*whl
-    python ${PADDLE_ROOT}/tools/print_signatures.py paddle.fluid > new.spec

-    if [ "$1" == "cp35-cp35m" ] || [ "$1" == "cp36-cp36m" ] || [ "$1" == "cp37-cp37m" ]; then
+    spec_path=${PADDLE_ROOT}/paddle/fluid/API_${spec_kind}.spec 
+    python ${PADDLE_ROOT}/tools/print_signatures.py paddle.fluid > $spec_path
+    if [ "$1" == "cp35-cp35m" ] || [ "$1" == "cp36-cp36m" ] || [ "$1" == "cp37-cp37m" ]; then 
        # Use sed to make python2 and python3 sepc keeps the same
-        sed -i 's/arg0: str/arg0: unicode/g' new.spec
-        sed -i "s/\(.*Transpiler.*\).__init__ (ArgSpec(args=\['self'].*/\1.__init__ /g" new.spec
-    fi
-
-    python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API.spec new.spec
+        sed -i 's/arg0: str/arg0: unicode/g' $spec_path
+        sed -i "s/\(.*Transpiler.*\).__init__ (ArgSpec(args=\['self'].*/\1.__init__ /g" $spec_path
+    fi   

+    # TODO(paddle-dev): remove op_use_default_grad_op_maker.spec 
    # Currently, we only check in PR_CI python 2.7
-    if [ "$SYSTEM" != "Darwin" ]; then
-      if [ "$1" == "" ] || [ "$1" == "cp27-cp27m" ] || [ "$1" == "cp27-cp27mu" ]; then
-        python ${PADDLE_ROOT}/tools/diff_use_default_grad_op_maker.py ${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_op_maker.spec
-      fi
+    if [ "spec_kind" == "PR" ]; then
+        if [ "$SYSTEM" != "Darwin" ]; then
+            if [ "$1" == "" ] || [ "$1" == "cp27-cp27m" ] || [ "$1" == "cp27-cp27mu" ]; then
+                python ${PADDLE_ROOT}/tools/diff_use_default_grad_op_maker.py \
+                    ${PADDLE_ROOT}/paddle/fluid/op_use_default_grad_op_maker.spec
+            fi
+        fi
    fi
    deactivate
 }

+
 function assert_api_spec_approvals() {
    /bin/bash ${PADDLE_ROOT}/tools/check_api_approvals.sh
    if [ "$?" != 0 ];then
@ -966,7 +1015,7 @@ function build_document_preview() {


 function example() {
-    pip install /paddle/build/python/dist/*.whl
+    pip install ${PADDLE_ROOT}/build/python/dist/*.whl
    paddle version
    cd ${PADDLE_ROOT}/python/paddle/fluid
    python sampcd_processor.py cpu 
@ -978,7 +1027,7 @@ function example() {


 function main() {
-    local CMD=$1
+    local CMD=$1 
    local parallel_number=$2
    init
    case $CMD in
@ -987,10 +1036,11 @@ function main() {
        build ${parallel_number}
        ;;
      build_and_check)
+        generate_upstream_develop_api_spec ${PYTHON_ABI:-""} ${parallel_number}
        cmake_gen ${PYTHON_ABI:-""}
-        build ${parallel_number}
-        assert_api_not_changed ${PYTHON_ABI:-""}
+        build ${parallel_number} 
        example
+        generate_api_spec ${PYTHON_ABI:-""} "PR" 
        assert_api_spec_approvals
        ;;
      build)
@ -1046,7 +1096,7 @@ function main() {
        run_brpc_test
        ;;
      assert_api)
-        assert_api_not_changed ${PYTHON_ABI:-""}
+        generate_upstream_develop_api_spec ${PYTHON_ABI:-""} ${parallel_number}
        assert_api_spec_approvals
        ;;
      test_inference)
--- a/tools/check_api_approvals.sh
+++ b/tools/check_api_approvals.sh
@ -5,7 +5,6 @@ fi

 PADDLE_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}")/../" && pwd )"
 API_FILES=("CMakeLists.txt"
-           "paddle/fluid/API.spec"
           "paddle/fluid/op_use_default_grad_op_maker.spec"
           "paddle/fluid/framework/operator.h"
           "paddle/fluid/framework/tensor.h"
@ -42,15 +41,21 @@ if [[ $git_files -gt 19 || $git_count -gt 999 ]];then
  fi
 fi    

+api_spec_diff=`python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec  ${PADDLE_ROOT}/paddle/fluid/API_PR.spec` 
+if [ "$api_spec_diff" != "" ]; then
+    APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 2 46782768 7534971 14105589 12605721 3064195 328693 47554610 39645414 11195205 20274488 45024560`
+    failed_num=`expr $failed_num + 1`
+    echo_line="You must have two RD (XiaoguangHu01 or wanghaoshuang or guoshengCS or heavengate or kuke or Superjomn or lanxianghit or cyj1986 or hutuxian or frankwhzhang or nepeplwu) approval for the api change for the management reason of API interface and API document."
+    echo_list=(${echo_list[@]}$failed_num "." $echo_line)
+fi
+
 for API_FILE in ${API_FILES[*]}; do
  API_CHANGE=`git diff --name-only upstream/$BRANCH | grep "${API_FILE}" | grep -v "/CMakeLists.txt" || true`
  echo "checking ${API_FILE} change, PR: ${GIT_PR_ID}, changes: ${API_CHANGE}"
  if [ "${API_CHANGE}" ] && [ "${GIT_PR_ID}" != "" ]; then
      # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable.
      # approval_user_list: XiaoguangHu01 46782768,chengduoZH 30176695,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,tensor-tang 21351065,xsrobin 50069408,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442. 
-      if [ "${API_FILE}" == "paddle/fluid/API.spec" ];then
-        APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 2 46782768 7534971 14105589 12605721 3064195 328693 47554610 39645414 11195205 20274488 45024560 `
-      elif [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then
+      if [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then
        APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 32832641 6836917`
      elif [ "${API_FILE}" == "CMakeLists.txt" ];then
        APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 6836917 46782768 30176695`
@ -63,11 +68,7 @@ for API_FILE in ${API_FILES[*]}; do
      fi
      echo "current pr ${GIT_PR_ID} got approvals: ${APPROVALS}"
      if [ "${APPROVALS}" == "FALSE" ]; then
-        if [ "${API_FILE}" == "paddle/fluid/API.spec" ];then
-          failed_num=`expr $failed_num + 1`
-          echo_line="You must have two RD (XiaoguangHu01 or wanghaoshuang or guoshengCS or heavengate or kuke or Superjomn or lanxianghit or cyj1986 or hutuxian or frankwhzhang or nepeplwu) approval for the api change! ${API_FILE} for the management reason of API interface and API document.\n"
-          echo_list=(${echo_list[@]}$failed_num "." $echo_line)
-        elif [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then
+        if [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then
          failed_num=`expr $failed_num + 1` 
          echo_line="You must have one RD (sneaxiy (Recommend) or luotao1) approval for op_use_default_grad_op_maker.spec, which manages the grad_op memory optimization.\n"
          echo_list=(${echo_list[@]}$failed_num "." $echo_line)
@ -134,5 +135,10 @@ if [ -n "${echo_list}" ];then
  git diff -U0 upstream/$BRANCH |grep "+" |grep -v "PADDLE_ENFORCE_" |grep "PADDLE_ENFORCE"
  echo "There are ${failed_num} approved errors."
  echo "****************"
+fi
+
+python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec  ${PADDLE_ROOT}/paddle/fluid/API_PR.spec
+
+if [ -n "${echo_list}" ]; then
  exit 1
 fi
--- a/tools/diff_api.py
+++ b/tools/diff_api.py
@ -15,7 +15,7 @@ differ = difflib.Differ()
 result = differ.compare(origin, new)

 error = False
-print('API Difference is: ')
+diffs = []
 for each_diff in result:
    if each_diff[0] in ['-', '?']:  # delete or change API is not allowed
        error = True
@ -23,13 +23,16 @@ for each_diff in result:
        error = True

    if each_diff[0] != ' ':
-        print(each_diff)
+        diffs.append(each_diff)
+'''
+If you modify/add/delete the API files, including code and comment, 
+please follow these steps in order to pass the CI:

+  1. cd ${paddle_path}, compile paddle;
+  2. pip install build/python/dist/(build whl package);
+  3. run "python tools/print_signatures.py paddle.fluid> paddle/fluid/API.spec"
+'''
 if error:
-    print(
-        '''If you modify/add/delete the API files, including code and comment, please follow these steps in order to pass the CI:
-    1. cd ${paddle_path}, compile paddle;
-    2. pip install build/python/dist/(build whl package);
-    3. run "python tools/print_signatures.py paddle.fluid> paddle/fluid/API.spec"'''
-    )
-    sys.exit(1)
+    print('API Difference is: ')
+    for each_diff in diffs:
+        print(each_diff)