Metacello / metacello

Metacello is a package management system for Smalltalk
MIT License
87 stars 43 forks source link

Support private git repositories using credentials #534

Closed LinqLover closed 3 years ago

LinqLover commented 3 years ago

This PR aims to add support in Metacello for loading projects from private git repositories by specifying credentials. To do so, the URL for zipballs is updated to use api.github.com instead of github.com (since the latter does not support authentication), and some call chains into the MetacelloSqueakPlatform are extended by username/pass parameters which are specified using the existing siteUsername/sitePassword properties from MCGitBasedNetworkRepository/MCFetchGithubRepository.

For use in Squeak, the following patch which has not yet arrived in the Trunk is required: [squeak-dev] The Inbox: WebClient-Core-ct.126.mcz (both version + changeset).

Usage example:

MCGitHubRepository siteUsername: 'LinqLover' sitePassword: 'thisIsAnAccessToken'.
MCFetchGithubRepository siteUsername: 'LinqLover' sitePassword: 'thisIsAnAccessToken'

Metacello new
    baseline: 'MyPrivateRepository';
    repository: 'github://LinqLover/MyPrivateRepository/src';
    load.
"Before this patch, you would see a 'could not resolve' error here"
LinqLover commented 3 years ago

Hm, the ancestry is a total mess. @dalehenrich Can you give me any pointers on how I can commit my changes in the right way to this project? I tried to use Squot, which obviously was not a good idea.

dalehenrich commented 3 years ago

@LinqLover, the first step would be to start with latest version of the master branch for Metacello (it looks like your branch dates back to June of 2019 and there have been quite a few commit since then ...

Of course if you do a git pull origin master in your local git clone, you will get the same set of conflicts, but at least you will be on your local machine and can manually address things:

  1. You could try manually editing the conflicting .st files using a text editor ... the conflicts in the version file are difficult to manually merge, so it depends upon how important the Monticello history is to you... For me the git history is fine, but Monticello will need a new version created, so I would use the old version of the version file, then after all of the merge conflicts for Metacello-MC and Metacello-Platform.squeak have been addressed, I would load and resave those two packages so that they have a new Monticello created.
  2. Or, if you can easily create a changeset (or just use the package diffs) you can manually patch a fresh copy of the master branch ... which will have to massage, the conflicts for the .st files means that someone else has edited those particular files.

The project https://github.com/ThierryGoubier/GitFileTree-MergeDriver does a good job of automatically merging version file conflicts in your local clone and I highly recommend it ... If you use it is likely than only the .st conflicts will need to manually merged and the version conflicts will be automatically merged ...

LinqLover commented 3 years ago

@dalehenrich, thanks for the help. I have a changeset that includes all changes (it's a quite small patch, no worries about merge conflicts), but I'm not familiar at all with the workflow that appears to be used for this project. I have only used Monticello and Squot in the past. How can I get my Smalltalk code into the filesystem to use it with git & Co.? Apparently, I am missing an essential tool here ...

dalehenrich commented 3 years ago

I am not familiar with Squot and I haven't used Squeak for many, many years:)

The basic workflow (that I think you are already using) is:

  1. fork the Metacello repostory.
  2. clone the Metacello repository to your local machine.
  3. attach the clone to your working image using a filetree:// path.
  4. load Metacello from the filetree repository using a Metacello new load expression.
  5. save your edits to the git repo and do a git commit
  6. push your commit to your local fork
  7. open a pull request ...

It seems to me that you've done all of these steps, the only misstep along the way, is that you based your work on an older version of the Metacello master branch ....

So for the long term maintenance of your fork, I would add that you should do periodic pull requests from your fork to pick up the latest commits on the master ... after the PR is complete, you would do a pull origin master in your local clone and update the code in your working image by loading the code into your image ... Picking up the latest work is important if you are planning on sharing your work in the original repository ...

I think that the only real thing that was missing from your workflow is that you should have https://github.com/ThierryGoubier/GitFileTree-MergeDriver installed on your development machine so the whrn you do your local pull origin master the monticello meta data updates would be automatically merged ...

Does this help?

LinqLover commented 3 years ago

Whoops, this happened because of my force-push.

LinqLover commented 3 years ago

Ah! Now the diff looks better, thanks a lot! IMHO this workflow should be documented in some readme file in this repository. :-)

dalehenrich commented 3 years ago

pull requests for documentation improvements are gladly accepted ...

LinqLover commented 3 years ago

@dalehenrich Do you have some experience with GemStone? I cannot load the GemStone platform code into Squeak so I edited it in a file editor, but despite https://github.com/Metacello/metacello/blob/858672969890f17043bf37263dcb7d190cbe5265/repository/Metacello-Platform.gemstone.package/MetacelloGemStonePlatform.class/instance/downloadZipArchive.to.username.pass..st and https://github.com/Metacello/metacello/blob/858672969890f17043bf37263dcb7d190cbe5265/repository/Metacello-Platform.gemstone.package/MetacelloGemStonePlatform.class/methodProperties.json all GemStone jobs do not find the MetacelloGemStonePlatform implementation of #downloadZipArchive:to:username:pass:. Is there some extra specification required for GemStone?

dalehenrich commented 3 years ago

@LinqLover, my guess is that this is a Monticello load problem ... with filetree, you can edit the individual methods in a package, but unless you update the Monticello version number, the "new" code will not be loaded.... Another argument for metadataless projects:)

Anyway, I have a command implemented in tode that will bump the monticello version number, so I could fork and clone your repo, checkout your branch and bump the version and then give you a pull request with the new version ... Let me know and I will do it this afternoon when I have more time ... (if I don't hear from you, I'll go ahead an do it, assuming that you've gone to bed:)

LinqLover commented 3 years ago

Thanks, you'll help me a lot if you could do this.

Just another todo before merging: Probably we want to see tests for this. Even for Squeak (which I manually tested the feature for), the authentication does not work yet without installing a changeset I have proposed to the list (if the changeset is rejected, I probably will need to add a few lines in this PR instead). I do not yet know which platforms else support the authentication out of the box. The test setup could be like this: Set siteUsername and sitePassword provided as Travis CI secrets, try to download any baseline from GitHub (which might be publicly available, too), and reset username/password. However, this would require storing extra secrets in Travis CI (which probably one of the repo administrators would have to create using a GitHub access token). What do you think?

dalehenrich commented 3 years ago

@LinqLover, I'm sorry I wasn't able to get to this this afternoon ... got hung working on fixing bugs for an upcoming release (and I'm on critical path:) ... I'll try to find time tomorrow ...

dalehenrich commented 3 years ago

Haha, after pressing Comment I thought for a minute and realized that I could get this out of the way pretty quickly by forking into GsDevKit (I'll delete the fork when the pull request is completed ...)

dalehenrich commented 3 years ago

pull request is ready when you are ...

LinqLover commented 3 years ago

Sorry for the delay. Thanks for your help! I'll merge https://github.com/LinqLover/metacello/pull/1 as soon as the last test passes (just restarted it).

However, I would not have a good feeling if we got this PR merged before there are any tests. Unfortunately, I'll be on vacation for the next week, so probably this PR will have to wait a while ... After that, I'm looking forward to your feedback on my test setup proposal from above. :-)

dalehenrich commented 3 years ago

okay ... ping me when you ready for my review ...

LinqLover commented 3 years ago

... just in midst of writing the tests ... FYIO, I'm burning lots of CI time & my time because the builds do not get interrupted automatically if a new commit is pushed. You could also turn on automatical build aborts in the Travis CI settings. :-)

LinqLover commented 3 years ago

Yahoo, the tests (including the new one) are finally green! @dalehenrich This should be ready for review now. I'm afraid this PR adds a quite large footprint to the ancestry because I needed to do some CI debugging. Do you see the need to clean it up separately or will it be enough to squash when merging? Looking forward to your review! :-)

LinqLover commented 3 years ago

Cool, thanks for merging! If someone needs support for Pharo/Gemstone, we can open another issue for it.

dalehenrich commented 3 years ago

My pleasure, I'm happy to see other folks contributing (especially from the Squeak community)!