UMPsychMethodsCore / MethodsCore

All of the projects that the methods core develops, combined into one repository!
7 stars 0 forks source link

CurrentVersionSHA fixes #51

Closed dankessler closed 12 years ago

dankessler commented 12 years ago

The post-checkout hook doesn't do a very stable job of updating the .local/CurrentVersionSHA file.

There seem to be two problems

  1. The path to the directory used by the checkout.sh script is based on a pwd but should use some of the trickier dirname $0 business used in the self_update.sh script.
  2. If you go to detached HEAD state, HEAD will not contain a reference to a branch, but rather the sha itself, so we need a way to identify the difference between these two things. My simple guess is that refs will include file separators, whereas sha's will not. Maybe we can use that?
mangstad commented 12 years ago

Since this is still open I'm guessing this isn't working in the current develop? Just asking because the logging code will depend on this since it is intended to embed the current version SHA in the logged script.

Also, any thought about how to deal with Robert's suggestion awhile back in issue #27 about a human readable version number for major releases? Do we just add another file in .local with the major release number, or just include that also in the CurrentVersionSHA file or something else.

mangstad commented 12 years ago

@dankessler do you see any downside to declaring mcRoot as a global variable so we can access it in any of our methods core functions without needing to pass it in? So instead of

%DEVSTART
mcRoot = fullfile(fileparts(mfilename('fullpath')),'..');
%DEVSTOP

in the templates it would be

global mcRoot;
%DEVSTART
mcRoot = fullfile(fileparts(mfilename('fullpath')),'..');
%DEVSTOP
dankessler commented 12 years ago

I don't see any issues with that

On Thu, Jun 14, 2012 at 2:14 PM, Mike Angstadt < reply@reply.github.com

wrote:

@dankessler do you see any downside to declaring mcRoot as a global variable so we can access it in any of our methods core functions without needing to pass it in? So instead of

%DEVSTART
mcRoot = fullfile(fileparts(mfilename('fullpath')),'..');
%DEVSTOP

in the templates it would be

global mcRoot;
%DEVSTART
mcRoot = fullfile(fileparts(mfilename('fullpath')),'..');
%DEVSTOP

Reply to this email directly or view it on GitHub:

https://github.com/UMPsychMethodsCore/MethodsCore/issues/51#issuecomment-6336365

Daniel A. Kessler Research Area Computer Specialist Psychiatry - Rachel Upjohn Building University of Michigan, Ann Arbor kesslerd@umich.edu +1 734.418.8134

dankessler commented 12 years ago

Also RE your earlier issue, yes, this is still an open issue. However, it's more of a weakness that I realized and wanted to follow-up on. For the most part, the deployed repositories should never be in an detached head state, and it's unlikely that the robustness I proposed in 1) is really necessary. You should be able to assume that stuff works and is in place for now.

heffjos commented 12 years ago

For number 2, my solution looks for a colon in .git/HEAD. I think the colon should only appear when HEAD points to a branch. Let me know if this is a good solution. Here is the added code to checkout.sh:

#Check if in detached head state
headContent=`cat .git/HEAD`
colonLoc=`expr index "$headContent" :`

#Identify Current Version
if [ $colonLoc -eq 0 ]
then
    cat .git/HEAD > .local/CurrentVersionSHA
else
    cat .git/`cat .git/HEAD | awk '{print $NF'}` > .local/CurrentVersionSHA
fi
dankessler commented 12 years ago

@heffjos that looks like a reasonable assumption and a good bit of code. Have you tried it?

heffjos commented 12 years ago

I tested it. The code updates the the current version sha correctly except I found some other behavior that may be bugs. I tested it by cloning my whole repository and then running setup_deployment.sh. When I did this, CurrentVersionSHA was not created until I actually performed a checkout. This can be fixed by putting git checkout HEAD at the end. I was just wondering if the installation I performed would be a typical one so that this line of code is actually needed. Committing anything does not update CurrentVersionSHA, but I do not think anyone should be committing in a deployment version.

Also, if the detached head is at a commit before my changes, then the updated code will be missing.

dankessler commented 12 years ago

Yeah the logic gets rather tricky. I think the git checkout HEAD idea doesn't hurt.

On Wed, Jun 27, 2012 at 3:57 PM, heffjos < reply@reply.github.com

wrote:

I tested it. The code updates the the current version sha correctly except I found some other behavior that may be bugs. I tested it by cloning my whole repository and then running setup_deployment.sh. When I did this, CurrentVersionSHA was not created until I actually performed a checkout. This can be fixed by putting git checkout HEAD at the end. I was just wondering if the installation I performed would be a typical one so that this line of code is actually needed.

Also, if the detached head is at a commit before my changes, then the updated code will be missing.


Reply to this email directly or view it on GitHub:

https://github.com/UMPsychMethodsCore/MethodsCore/issues/51#issuecomment-6613833

Daniel A. Kessler Research Area Computer Specialist Psychiatry - Rachel Upjohn Building University of Michigan, Ann Arbor kesslerd@umich.edu +1 734.418.8134

heffjos commented 12 years ago

I add git checkout HEAD: heffjos@934226c

dankessler commented 12 years ago

Sweet. I've tweaked the reference for your commit so that it points more clearly to a commit in your repo rather than mine.

Also, would you mind potentially putting these changes in a separate branch, and resetting your develop back so it doesn't include these? Would make it easier to figure out where the changes are if I fetch from you.

heffjos commented 12 years ago

New branch is SHA_fix.

dankessler commented 12 years ago

Sweet, thanks!

On Thu, Jun 28, 2012 at 12:01 PM, heffjos < reply@reply.github.com

wrote:

New branch is SHA_fix.


Reply to this email directly or view it on GitHub:

https://github.com/UMPsychMethodsCore/MethodsCore/issues/51#issuecomment-6633752

Daniel A. Kessler Research Area Computer Specialist Psychiatry - Rachel Upjohn Building University of Michigan, Ann Arbor kesslerd@umich.edu +1 734.418.8134

dankessler commented 12 years ago

@heffjos does your code address the first issue I raised in the issue description at all?

dankessler commented 12 years ago

Looks like part 1 may actually be a non issue.

From http://longair.net/blog/2011/04/09/missing-git-hooks-documentation/

post-checkout

(Tested with “git commit” in a working tree – it cannot be used with a bare repository.)

The current directory is the top level of the working tree....

So it looks like the attempt to identify mcRoot using pwd, viz.

mcRoot=`pwd`

at the top of checkout.sh is kosher. However, I've added a comment to both checkout.sh and post-checkout (which calls it) to make it clear why we can get away with this.

However, the greater caution, as exercised in self_update.sh is warranted, since I don't know how that will get called (could be via cron, or manually).

dankessler commented 12 years ago

And now changes made by @heffjos, plus some of my own, are incorporated in my SHA_fix branch. Submitting for pull request presently.