Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding the ability for users to override implicitly loaded global libraries. #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

willcrain1
Copy link

In my company we use this shared pipeline library plugin, and I find it hard to test changes to shared pipeline libraries unless I have write access to their repository and am able to create a branch, then I can refer to the changes with @Library('libraryname@branchname').

Currently it seems there's no way to import a fork of a library to override an implicitly loaded shared library. I'd use this functionality frequently to test any changes made in our shared libraries.

I was able to test this PR by building hpi files and importing them into the jenkinsci official docker container. I then created a shared library here: https://github.com/willcrain1/test-pipeline-library and forked it here as well and made minor changes: https://github.com/jenkinspipelinetesting/test-pipeline-library

I executed the libraries in this Jenkinsfile: https://github.com/willcrain1/test-pipeline/blob/master/Jenkinsfile

@jglick @kohsuke any feedback is appreciated.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting aside technical details about the code here, I am not sure this feature is safe. There is indeed an unmet need

there's no way to import a fork of a library

but I think this should be addressed at a different layer, such as in the github-branch-source implementation of the relevant API.

@@ -105,6 +106,15 @@ public LibraryRetriever getRetriever() {
public Boolean getChangelog() {
return changelog;
}

public Boolean getExistingLibrariesUsed() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getters and setters should take boolean, not Boolean.

@@ -89,6 +89,7 @@
private final String identifier;
private Boolean changelog = true;
private LibraryRetriever retriever;
private Boolean existingLibrariesUsed = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a risky default.

@jglick
Copy link
Member

jglick commented May 23, 2022

As of #172 most of the code in this plugin has been moved to another plugin repository so this PR must be closed. If this change is still needed, please

git clone https://github.com/jenkinsci/pipeline-groovy-lib-plugin
cd pipeline-groovy-lib-plugin
git checkout -b PLEASE-SELECT-BRANCH-NAME
git pull https://github.com/willcrain1/workflow-cps-global-lib-plugin master

resolve any merge conflicts, and file a fresh PR on the new repository. Be sure to paste a link to this old PR to enable bidirectional navigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants