Skip to content

Gatekeeper Merging Pull Request

Jump to bottom
dpefour edited this page Mar 23, 2015 · 6 revisions

Important information about Git branch in this document

Although most of the contributions typically target the master branch, the actual branch accepting contributions depends on the Vivado release and needs to be checked on this page.

In this document, the Git branch can be refered as master or <branch> but the exact Git branch name should be used for all the Git commands of this document.

Gate Keeper Steps for Merging a Pull Request

Once all the App Owner(s) associated with a pull request gave their blessing, the Gate Keeper can merge the pull request inside the master branch of the Xilinx repository.

Make sure you are set up as a normal contributor (name/email/proxy config settings)

To prevent corruption of your working area, it is recommended to perform the following steps inside an empty directory. The directory can be deleted once the pull request has been tested.

  1. The gatekeeper owner must first verify that the pull request includes the digitally signed doc/legal.txt file by the contributor(s) of the pull request. Otherwise the pull request cannot be merged for legal reasons.

Any new proc for the app should also have a regression test under ./test and, if possible, a detailed documentation under ./doc .

  1. Create a repository by cloning XilinxTclStore into a temporary directory.
On Windows
git clone https://github.com/Xilinx/XilinxTclStore.git
On Linux
git clone https://<USER>@github.com/Xilinx/XilinxTclStore.git
  1. Enter the repository that has been just checked out
cd XilinxTclStore
  1. Rename and add the Git remotes to make the name more concistent with their traditional definitions:
git remote rename origin upstream
git remote add origin https://<USER>@github.com/<USER>/XilinxTclStore.git

Verify the Git remotes:

git remote -v

The remote origin should point to your GitHub repository and the remote upstream should point to Xilinx GitHub repository.

  1. Check out the branch <branch> that needs to be worked on:
git checkout <branch>
  1. Merge the pull request. This step is based on the pull request number that is unique and incremental for each pull request.
git pull upstream refs/pull/<pull_request_number>/head

For example, if the pull request number is 173:

git pull upstream refs/pull/173/head

  1. Verify that there was no merge conflict. Any conflict must be resolve before proceeding.

  2. The list of modified/added files from the pull request can be obtained with the following Git command:

git diff --name-status HEAD HEAD~1

For example:

% git diff --name-status HEAD HEAD~1
M       tclapp/xilinx/designutils/convert_muxfx_to_luts.tcl
M       tclapp/xilinx/designutils/pkgIndex.tcl
M       tclapp/xilinx/designutils/tclIndex
M       tclapp/xilinx/designutils/test/src/convert_muxfx_to_luts/convert_muxfx_to_luts_0001.dcp
M       tclapp/xilinx/designutils/test/test.tcl
M       tclapp/xilinx/designutils/utils.tcl

% git diff --name-only HEAD HEAD~1
tclapp/xilinx/designutils/convert_muxfx_to_luts.tcl
tclapp/xilinx/designutils/pkgIndex.tcl
tclapp/xilinx/designutils/tclIndex
tclapp/xilinx/designutils/test/src/convert_muxfx_to_luts/convert_muxfx_to_luts_0001.dcp
tclapp/xilinx/designutils/test/test.tcl
tclapp/xilinx/designutils/utils.tcl
  1. Now that the code from the pull request has been merged, the next steps are to verify that the app regression test is not broken and that app version and catalog have been updated
  • The Vivado linter must be run on the app scripts
  • The app test suite must be run to verify that the code does not break the regression tests
  • The app version should have been incremented unless special reason not to do it (check with app owner)
  • The release catalog XML needs to be updated (app revision number and commit id)
  1. Check the app revision number

If the app version is not incremented, Vivado won't be able to pull the new changes from GitHub. If the app version has not been incremented, check with the app owner whether this was the intent or not

The app version is coded inside the Tcl file under the app directory that provides the Tcl package:

package provide ::tclapp::<COMPANY>::<APP> <MAJOR_VERSION>.<MINOR_VERSION>

Only the MINOR_VERSION should be typically incremented. The MAJOR_VERSION should be incremented only when the changes inside the app are profund enough to break the backward compatibility.

For example:

package provide ::tclapp::xilinx::designutils 1.4

Refer to [Create the Package Provider] (Adding-a-New-App-to-the-Repository) for additional information regarding the app version.

If and only if the app revsion number has been incremented, the release catalog XML must be updated:

tclapp::update_catalog mycompany::myapp

The release XML can also be updated for multiple apps at once by providing the list of apps:

Vivado% tclapp::update_catalog mycompany1::myapp1 mycompany2::myapp2 ... mycompanyN::myappN
  1. Start Vivado from a different directory and point to the temporary directory where the Xilinx repo has been checked out and the pull request merged:
shell% setenv XILINX_TCLAPP_REPO <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore
shell% setenv XILINX_LOCAL_USER_DATA NO
shell% vivado -nojournal -nolog
  1. If the app is not already installed, it can be installed from the command line with the ::tclapp::install command

Use the ::tclapp::install command to install your app. If the app is already installed, the command will un-install the app first before re-installing it:

::tclapp::install <COMPANY>::<APP>
For example:
::tclapp::install xilinx::designutils

Note: the app can also be uninstalled/installed from the GUI.

  1. Verify that all the expected procs are visible and accessible under the <COMPANY>::<APP> namespace

If some procs are missing, it is most likely because they have not been exported inside the Tcl script where they have been defined.

The command line arguments should also be tested through the help system:

<COMPANY>::<APP>::<PROC> -help
For example:
xilinx::designutils::write_template -help

The list of arguments should be verified as well as the description and return value of each procedure.

  1. Open the GUI and check the list of procs displayed under the app. If some of the exported procs are missing, this is most likely due to the catalog XML that has not been rebuilt.

  2. Run Vivado linter on the app

vivado% lint_files [glob <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/*.tcl]
  1. Source all the scripts under the app just to make sure that they don't contain some errors that would prevent the app from installing
vivado% foreach file [glob <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/*.tcl] { source -notrace $file }
  1. Run the app test suite
vivado% source <ABSOLUTE_PATH_TO_TEMPORARY_DIRECTORY>/XilinxTclStore/tclapp/mycompany/myapp/test/test.tcl

  1. Exit Vivado

  2. Add the updated files to Git and commit the changes

The catalog(s) XML should be the only files to be updated unless a change was made by the gatekeeper in other file(s)

git status
git add <LIST_OF_NEW_AND_UPDATED_FILES>
git commit -m 'Merge pull request 173 and updated catalog(s) XML'
  1. Push your changes directly to Xilinx GitHub repository
git push upstream <branch>
  1. Push your changes to your GitHub repository so that it is in sync
git push origin <branch>
  1. Close the pull request if not already done by GitHub and remove the temporary directory where the Xilinx repo has been checked out
Clone this wiki locally