Metacello / metacello

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

Support credentials parameters in the Scripting API #536

Closed LinqLover closed 4 years ago

LinqLover commented 4 years ago

With this PR, two new parameters are added to the Metacello Scripting API, #username and #password, which allow overriding the credentials for the specified fetch operation temporarily. Example usage:

Metacello new
    baseline: 'External';
    repository: 'github://LinqLover/MetacelloTest-External/repository';
    username: 'myfancyusername';
    password: 'mycoolpassword';
    load.

The new parameters should at least work for GitHub + BitBucket repositories using Squeak (see also MetacelloRepositorySqueakCommonTestCase >> testGetGithubRepositoryScriptingAPI), but HTTP/FTP repositories are expected to work as well (though I did not test it). Other Smalltalk platforms are not supported at the moment; analogously to https://github.com/Metacello/metacello/pull/534#issuecomment-718079325, I'd propose adding support for them on-demand only. (I have no how Iceberg works (not to speak of Gemstone at all), so this would probably go beyond my skills at the moment ...)

Looking forward to your review! :-)

dalehenrich commented 4 years ago

@LinqLover on the off chance that you have more changes planned, I'll wait for a confirmation from you that you're done before merging ...

LinqLover commented 4 years ago

@dalehenrich Thanks for your review! What does not yet work is specifying credentials for the loading of entire dependency trees, e.g. given a private GitHub package A that requires a private GitHub package B via baseline. I'm not yet sure how this would be best implemented, probably via another global variable plus an additional #recursiveAuthArg in the Metacello Scripting API, but I do not yet have a proper overview of all authenticable sources that can be used with Metacello (see https://github.com/Metacello/metacello/issues/538#issuecomment-723488631).

However, I think this change would fit well into a second PR, so unless you have further objections, please feel free to merge this one! :-)

dalehenrich commented 4 years ago

@LinqLover, before pulling the trigger on this ... have you seen that github is deprecating password authentication ... I can go ahead and merge this PR, but if github will be deprecating password authentication, it may be that it is a little late to add this support? I have not looked into the replacements: personal access tokens or the web application flow ...

LinqLover commented 4 years ago

Ah yes, I was already aware of this ... It is only a naming issue. You can pass an access token as #password as well (which even works without specifying a username). I did not want to rename the selectors for reasons of compatibility/uniformity. Do you think I should add some explaining comments about access tokens?

dalehenrich commented 4 years ago

@LinqLover comments would be a good idea ... I didn't know the deal with access tokens, but now I do:)

LinqLover commented 4 years ago

Sorry for the long delay ... Now there are finally comments. :-)

PS: I just noticed that git apparently has converted some line endings because I was committing using Windows. Is this a problem for filetree? Otherwise, PR is ready from my side!

LinqLover commented 4 years ago

@dalehenrich Any updates on this? I see I have repository access (thank you!), but I would prefer only to merge this after you have approved the latest changes. :-)

LinqLover commented 4 years ago

Thank you! :-)

dalehenrich commented 4 years ago

@LinqLover I'm glad to see you contribute to the project and it is much appreciated ... BTW, with regards to triggering a CI build, in case you didn't know, there is a more options menu on the travisCI site for each project that you can use to trigger a build for given branch and custom commit comment ... saves a trigger ci commit:) ... I used that feature to trigger this travis build

LinqLover commented 4 years ago

Yes, I knew this trigger build button - pretty helpful, I already used it for this PR! But in this case (https://github.com/LinqLover/smalltalkCI/commit/f63a56467434f6007bf96a23349ca0170670dabb) I needed to retrigger the CI for my PR at hpi-swa/smalltalkCI for which I don't have administrative access rights to Travis ... :-)