astropy / package-template

Template for packages that use Astropy. Maintainer: @astrofrog
http://docs.astropy.org/projects/package-template/en/latest/
Other
60 stars 62 forks source link

Add instructions on how to update an existing affiliated package that already follows package-template. #23

Closed weaverba137 closed 9 years ago

weaverba137 commented 11 years ago

Maybe these instructions already exist, but what is the procedure for updating an affiliated package that already adheres to the package-template standard when changes to package-template come out?

eteq commented 11 years ago

In principal the instructions are just to do git merge package-template master (assuming "package-template" is a remote you have pointing to astropy/package-template). But you're right we should have instructions on how to do this, particularly tips when things go wrong (e.g. merge conflicts).

weaverba137 commented 11 years ago

Does git merge package-template master assume that the affiliated package was created from a fork of package-template?

weaverba137 commented 11 years ago

I tried this:

git remote add template git://github.com/astropy/package-template.git
git fetch template

but the merge didn't work:

$ git merge template/master astropy
Unable to find common commit with template/master
Automatic merge failed; fix conflicts and then commit the result.
$ git merge --abort
$ git merge template astropy
fatal: 'template' does not point to a commit

Note: currently, the branch astropy follows the package-template layout more closely than master.

astrofrog commented 11 years ago

@weaverba137 - which package are you trying to pull the latest template-package changes into?

(@eteq - this is the kind of issue I was talking about in the other issue)

weaverba137 commented 11 years ago

https://github.com/weaverba137/pydl

eteq commented 11 years ago

Ah, I think I may understand what's happening here. @weaverba137 , did you pull in the template files by copying-and-pasting the template files (or just copying them from elsewhere)? If so, then what I suggested won't work, because it doesn't have the git commit tree, and thus doesn't know how to update anything.

The way that would have allowed the merge to work the way I meant is if (when you first decided to pull in the astropy template) you had done something like

git branch astropy #I'm assuming this doesn't exist yet
git checkout astropy
git remote add template git://github.com/astropy/package-template.git
git fetch template
git merge template/master 

That would have pulled in all the template files and the git revision tree. From then on you could use git merge template/master to do the necessary incremental changes.

Does that make sense?

I completely understand why you didn't do this, as I don't think there's any documentation explaining this anywhere. So you have two options:

  1. Just by-hand copy-and paste changed parts of the package template when it gets updated.
  2. I've never tried this, but in theory it should work (famous last words...): First figure out which package-template commit corresponds to where you pulled in the template (and what it's SHA is - that's <SHA> in the next sentence). Then do a git checkout astropy followed by git merge <SHA>. That will produce a bunch of merge conflicts, but always choose your current version over the one that you're merging in. So then git will now think you're merged at the correct SHA. Then you should be able to do git merge template/master, and it should now do all the normal git things it's supposed to do to update for all the commits in between.

This certainly all needs to be documented, though. @astrofrog - what do you think about this situation?

weaverba137 commented 11 years ago

@eteq, that's pretty much what happened. For completeness, the exact sequence of events was this:

  1. I created a new GitHub repository for pydl (from an svn dump from the old repository). The master branch of pydl is still very close to this code.
  2. I created the astropy branch.
  3. I added files and directories from package-template to the astropy branch by hand (copying the files on disk).

I understand your instructions, but would they work if the new branch one created was not empty? I guess there would be a step where you dealt with the conflicts that the initial merge with template/master created.

eteq commented 11 years ago

@weaverba137 - yep, I've tried that before even with a non-empty branch and it works. It's a bit non-standard, because then the version tree has two roots. (Or perhaps tips? not sure which part of the tree is "up"...) But git seems to still manage to figure out how to merge for the test cases I've done. As you say, though, it does require dealing with a bunch of conflicts the first time. It's also possible that it would start getting confused with a package more complicated than the test cases I've used. But I find git is often surprisingly smart about figuring out where to merge things.

It also means that if something in the template gets changed that's an intended change on your side. For example, if we changed the default license, and you had already changed to something other than the default license, you'd have to know to ignore that change because git would register it as a merge conflict (not knowing that your version is the one it should trust). I think there's a way to always tell git to pick your side, but it's probably not really worth the trouble of figuring that out unless you do it a lot.

weaverba137 commented 11 years ago

@eteq, I tried the git merge <SHA>, and it seems to have worked.

eteq commented 11 years ago

Ok, great - hopefully that will make it easier to merge in the future.

I guess this issue is now a guide to anyone who wants to write these instructions, then :)

mperrin commented 10 years ago

Just wanted to chime in here and say thanks. This is indeed useful guidance and I've been following it to get my poppy and webbpsf packages connected to the affiliated package template so I don't have to keep manually copying over changes to the template.

It's great to be able to do this for cases where the affiliated package wasn't started from a fork of the repository, but instead existed on its own at first before getting affiliated...

eteq commented 10 years ago

Well, @mperrin, I can't help but think that means you're an excellent candidate for actually putting these instructions in the README of this repo ;)

cdeil commented 10 years ago

@eteq Could you please take the time to put these instructions in the docs? (I'll have to do this soon for https://github.com/gammapy/gammapy/issues/126)

cdeil commented 10 years ago

I finally decided to try and update gammapy from package-template (see https://github.com/gammapy/gammapy/pull/126) using the instructions that are currently in the README . One issue I just now noticed is that the following files were added to the gammapy repo which shouldn't have happened:

 create mode 100644 packagename/_astropy_init.py
 create mode 100644 packagename/tests/coveragerc
 create mode 100644 packagename/tests/setup_package.py

I don't know if I made a mistake during the conflict resolution or if the instructions are incorrect.

astrofrog commented 10 years ago

These files should indeed be added to the repository when updating since they were added to the package-template, but you need to move them with:

git mv packagename/_astropy_init.py gammapy/_astropy_init.py
...

Does that make sense? That way, next time _astropy_init.py is updated, it will go to the right place. We should document this.

cdeil commented 10 years ago

@astrofrog Yes, that makes sense. This time I might have done git rm packagename/_astropy_init.py (don't remember), and I definitly copied over _astropy_init.py manually after because I noticed python setup.py install didn't work because it was missing for gammapy.

Yes, for affiliated package maintainers like myself that don't know much about git and astropy-helpers, having good instructions would be gold.

The README for package-template is getting super-long and there's no way to link to bullets from discussions such as here. How about putting that info in Sphinx docs and add sections, so that one can link to and full-text search this information for affiliated package maintainers?

cdeil commented 10 years ago

Another issue I just noticed is that after this update @adonath is no longer in the list of contributors for Gammapy: https://github.com/gammapy/gammapy/graphs/contributors

But git shortlog -s -n shows that he has made 16 commits !?

mdboom commented 10 years ago

I was surprised that git had trouble with the moved directory, but having just done an experiment on some toy repos, it does in fact seem to work that way (that new files will end up in packagename). So we definitely should document this.

mdboom commented 10 years ago

As for @adonath not showing up in the list of contributors: are you sure it's a new thing? https://help.github.com/articles/i-don-t-see-myself-in-the-repository-contributors-graph suggests that if the email used in the git log does not match the email set in the github account, they won't show up in the contributor graph (which kind of makes sense).

cdeil commented 10 years ago

@mdboom - I know that at some point in the past the Github contributor list for Gammapy did show @adonath, but it could be that he changed his email associated with his Github account, so no, I'm not sure merging in from package-template had anything to do with it.

But GitHub and any other automatic contributors list and project history for Astropy affiliated package are completely incorrect anyways because of all the commits to the package-template that are merged in. E.g. according to Ohloh, Gammapy started in October 2011 and had 19 contributors, but actually that's when the package-template started and most contributors are for package-template.

So never mind, no point in trying to figure this one out.

eteq commented 10 years ago

Note that #98 will address this (I claim some credit, because he did it at my urging after we chatted about it :wink:)

weaverba137 commented 9 years ago

I think the astropy documentation covers this issue pretty well now, so I'd close this.

mwcraig commented 9 years ago

Closing per @weaverba137