search

TriBITSPub / TriBITS

TriBITS: Tribal Build, Integrate, and Test System,
http://tribits.org
Other
36 stars 46 forks source link

Automate install and update of local git hooks and commit template #138

Open bartlettroscoe opened 8 years ago

bartlettroscoe commented 8 years ago

Description:

Projects benefit from using local git commit hooks. Closely related to this is providing a default commit log message template (see trilinos/Trilinos#501).

Tasks:

  1. Learn and document the ins and outs of installing local git commit hooks and commit templates (see below) [Done]
  2. Develop a system for managing the install and updating of git commit hooks as a byproduct of running cmake.
bartlettroscoe commented 8 years ago

The site:

provides a nice overview of local git commit hooks. Basically, you just install hooks into the directory <repoDir>/.git/hooks/. Executable files with the names:

are automatically used as git hooks. That above website gives good detail about how these scripts are called, what they do, and gives good examples of how they are used.

It is recommended on that webpage to create symlinks to the hooks in your git repo. The disadvantage of a symlink is that the hooks will be missing if you checkout an older version of the repo. Therefore, we should not use symlinks. I would like to copy the files, but only if they need to be copied and only print that they are being copied when they are copied.

Also, it looks like adding a git commit message template is super easy as described here:

You just define a file called something like .git_commit_template.txt and then call:

$ git config commit.template $PWD/.git_commit_template.txt
bartlettroscoe commented 8 years ago

NOTE: See proposal for single function TRIBITS_INSTALL_LOCAL_GIT_HOOKS() below.

For installing git hooks, we need a function like:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS(
  <GIT_HOOKS_SRC_DIR>       # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR 
  <GIT_HOOKS_DIST_REPO_DIR> # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR
  )

To install the git commit template, we could use a function like:

TRIBITS_INSTALL_GIT_COMMIT_TEMPLATE(
  <GIT_TEMPLATE_FILE>  # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR
  <GIT_REPO_DEST_DIR>  # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR
  )

A TriBITS repo could then call these functions from the file <repoDir>/cmake/CallbackSetupExtraOptions.cmake as:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS( git_hooks . )
TRIBITS_INSTALL_GIT_COMMIT_TEMPLATE( git_hooks/git_commit_template.txt  . )

The file <repoDir>/cmake/CallbackSetupExtraOptions.cmake is a good file to put this in because this file only gets processed when a full configure is taking place, not when only partial processing is performed as part of dependency analysis.

TriBITS should install the file <GIT_TEMPLATE_FILE> under the directory <repoDir>/.git/hooks/ and then link to that. The reason for that is because if you check out an older version repo, you don't want this file to go missing.

TriBITS would then install the commit template by calling:

$ git config commit.template .git/hooks/git_commit_template.txt

in the working directory ${${REPOSITORY_NAME}_REPO_SOURCE_DIR}/., but only if a .git/ directory existed under that path.

A few issues to consider:

  1. If the directory .git/ does not exist, then the install is skipped (such as when configuring from the untarred tarball). This should likely be silent so that users don't see this. But we could add an option to allow developers to debug this (like ${PROJECT_NAME}_GIT_HOOKS_INSTALL_VERBOSE=TRUE).
  2. A user should be able to disable these installs by setting ${PROJECT_NAME}_GIT_HOOKS_INSTALL_SKIP=TRUE.
  3. Should use CONFIGURE_FILE() to copy the files into the ./git/hooks/ directory since it will copy them only if they are different. And print that the file is being copied only when it is being copied.
  4. Don't install an older version of the hooks when checking out an older version of the git repo. To accomplish this, TriBITS could write a file into ./git/hooks/ that lists the SHA1 and commit date for the updated files. So when you check out an older version of the repo, you would avoid updating the scripts. For example, a file git_hooks_version could be put into the .git/hooks/ dir which contains <SHA1> <COMMIT_DATE> You would also want to print when the update is not done because an older version of hooks were present (such as when checking out an older release branch).
  5. Some basic automated tests are needed to make sure these different use cases are working correctly and protect this functionality.

Doing this carefully and adding good unit tests will take some effort. But I think once this is done, it will be a very nice way to automatically install these hooks in seamless way for individual developers.

bartlettroscoe commented 8 years ago

I am now thinking to instead do one function:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS(
  HOOKS_SRC_DIR <GIT_HOOKS_SRC_DIR> # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR 
  REPO_DIR <GIT_HOOKS_DIST_REPO_DIR> # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR
  [INSTALL_COMMIT_TEMPLATE <template_file_name>]  # Relative to <GIT_HOOKS_SRC_DIR>
  )

This would then be used like:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS(
  HOOKS_SRC_DIR cmake/local_git_hooks 
  REPO_DIR .
  INSTALL_COMMIT_TEMPLATE git_commit_template.txt
  )

That way, we have just one function and yet we don't silently install the git commit template or hard-code the name of the git commit template file. Git itself hard-codes the names of the hook scripts 'commit-msg', 'pre-push', etc. so what have to go with that.

bartlettroscoe commented 8 years ago

Below is a long email chain with Brenan Kochunas dicussing this story. He implemented a verion of MPACT that I will then take and use as the foundation for the TriBITS version.

From: Kochunas, Brendan [mailto:bkochuna@umich.edu] Sent: Tuesday, August 09, 2016 3:44 PM To: Bartlett, Roscoe A Cc: casl-vri-infrastructure@casl.gov Subject: Re: [EXTERNAL] Adding git config install to configure

Ross,

Thanks for following up. Yes, I pushed this Friday. My apologies for not getting back to you sooner. The relevant commit on casl-dev:MPACT is:

commit c480d4254cb5530a9224f61fdef621a4c97adc9a
Author: Brendan Kochunas <bkochuna@umich.edu>
Date:   Fri Aug 5 10:40:17 2016 -0400

    Adding automatic installation of git commit template

    Description:
    Added new file MPACT_InstallGitCommitTemplate.cmake which defines a
    function that will install a git commit template. This function
    is called by the CallbackSetupExtraOptions which is called before package
    processing early in the configuration of a TriBITS project. For a discussion
    of this topic within TriBITS see:
    https://github.com/TriBITSPub/TriBITS/issues/138#issuecomment-233962167

    CASL Ticket # - N/A

I think the proposed compromise is fine. Please feel free to archive this email chain on the TriBITS github project. Thanks,

-Brendan

On 8/9/2016 3:00 PM, Bartlett, Roscoe A wrote: Hel Brendan,

I think you already pushed to the MPACT repo right? Has this synced with casl-dev/master yet? If so, what is the commit SHA1?

As for TRIBITS_INSTALL_LOCAL_GIT_HOOKS(), I think a nice compromise might be:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS(
  SRC_DIR <GIT_HOOKS_SRC_DIR>       # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR 
  REPO_DIR <GIT_HOOKS_DIST_REPO_DIR> # Relative to ${REPOSITORY_NAME}_REPO_SOURCE_DIR
  [INSTALL_COMMIT_TEMPLATE <template_file_name>]  # <template_file_name> relative to <GIT_HOOKS_SRC_DIR>
  )

so by default it would not silently install a git commit template but the project developer could explicitly ask for this with:

TRIBITS_INSTALL_LOCAL_GIT_HOOKS(
  SRC_DIR cmake/git_hooks
  REPO_DIR .
  INSTALL_COMMIT_TEMPLATE git_commit_template.txt
  )

That way, we don’t have to duplicate logic and arguments between these two functions. Is that a reasonable compromise?

Also, can I archive this email chain in:

?

This is a nice discussion of requirements that would be nice to document in the ticket.

Thanks,

-Ross

From: Kochunas, Brendan [mailto:bkochuna@umich.edu] Sent: Thursday, August 04, 2016 9:59 PM To: Bartlett, Roscoe A Cc: casl-vri-infrastructure@casl.gov Subject: Re: [EXTERNAL] Adding git config install to configure

Ross,

My responses in-line below.

On 8/4/2016 1:20 PM, Bartlett, Roscoe A wrote: Brendan,

From: Kochunas, Brendan [mailto:bkochuna@umich.edu] Sent: Thursday, August 04, 2016 12:32 PM To: Bartlett, Roscoe A Cc: casl-vri-infrastructure@casl.gov Subject: Re: [EXTERNAL] Adding git config install to configure

Thanks for the links Ross. I am fiddling with this while I'm waiting for things to build while debugging. I hadn't thought about the testing issues for this, and its not obvious to me how this would be done. Looks like I arrived at a similar approach that you outline. Briefly, what I am doing is:

  1. Create a new CMake macro (you suggest a function) that performs the following operations: • Verify the source directory is a repo • Verify the existence of a git executable • Check if the commit template is installed (git config --get commit.template) • If it's a git repo, git exists, and no template is installed, then install the template (git config --local commit.template <template_file>)

[Ross] I think you would want to install a new template if it was updated in the repo, even if an existing template was installed. That way the project leaders can update the template and have it automatically get updated on all developer repos. But if a developer does not want that template for some reason, then they can just configure with –D_GIT_HOOKS_INSTALL_SKIP=TRUE. Sound reasonable?

Yes this is a good point.

  1. Call this macro in the /cmake/CallbackSetupExtraOptions.cmake

I hadn't considered a defining a command line CMake argument, but that seems useful. Some other potential issues are how the template is installed e.g. global vs. local and dealing with the situation where another template might already be installed (globally or locally).

[Ross] For something automatic like this, I think it should be installed locally only. Also, I think each repo might want to have its own unique git commit message template. For example, for MPACT, you want people listing issue ticket IDs for PHI Kanban tickets and GitLab Issues, right?

I agree.

I guess what I see myself doing is prototyping your proposed TRIBITS_INSTALL_GIT_COMMIT_TEMPLATE function specifically for MPACT. Perhaps this would be of some value to TriBITS?

[Ross] Absolutely. Once you get this completed and working for MPACT, point me to it and I can copy into TriBITS proper. Then MPACT can drop its copy and use the one in TriBITS? Does that sound reasonable?

Absolutely. Although you may want to tweak a few things in the implementation of the function. I will likely push the modifications soon, once I do, I will let you know that this file is available and where it is.

[Ross] I think I can add a few automated tests that check behavior if .git/ exists or not, and other things without actually having to call git or have a real git repo. I can take care of that.

Great.

[Ross] Also, I am thinking that it might be better to just have a single function called TRIBITS_INSTALL_LOCAL_GIT_HOOKS() and then require the commit template file be named something like ‘git_commit_template.txt’ and carefully document that. I think that will be easier to maintain for everyone involved. Does that sound reasonable?

It sounds reasonable, but having two separate functions also sounds reasonable to me. In fact the TRIBITS_INSTALL_LOCAL_GIT_HOOKS() could be easily refactored to call the function for installing the commit template if the functions are initially separate. The GIT_HOOKS install could even be setup to check if the required template file name exists and call the template install function and you could still leave the commit template install function separate for a little more flexibility for TriBITS project developers.

I'm not sure what the best way is, but what I have implemented strictly installs a commit template and nothing more.

P.S. What do you think about a commit-msg hook like the one shown here:

https://github.com/trilinos/Trilinos/issues/501#issuecomment-235896919

I would love to see something like this in action. My experience is similar to yours that some developers forget about the blank line and 72 char limit. We sort of make these suggestions in the commit template, but having a hook provides guaranteed feedback that I think any developer would find annoying, so its likely to enforce well formatted commit messages.

On 8/4/2016 11:47 AM, Bartlett, Roscoe A wrote: Brendan,

We also want this for the Trilinos project as well. See:

I have a detailed plan for doing this with TriBITS described here:

that will work for Trilinos, MPACT, and any project that uses TriBITS.

If you have time and energy to help out with this, then we can do this soon. If you have any comments on what is proposed, please comment in:

The hard part will be writing quality automated tests in TriBITS to show that this works and to protect it. Actually implementing this should be pretty easy. But I guess if the git hooks don't get updated it is not the worst thing ever so we could go light on the testing perhaps (but I do want something).

Cheers,

-Ross

From: Kochunas, Brendan [mailto:bkochuna@umich.edu] Sent: Thursday, August 04, 2016 10:18 AM To: Bartlett, Roscoe A Subject: [EXTERNAL] Adding git config install to configure

Hi Ross,

We have a commit template for MPACT, and at one point I recall a brief discussion where it was suggested that we could have CMake install this template during the configure. I'd like to get this taken care of. Would you happen to know how to do this?

I'll be googling around in the mean time to see if I can figure out how to do this.

Thanks,

-Brendan