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

Added support for PERL_CPM_MIRROR to set default mirror #205

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

Conversation

christopherraa
Copy link

Adding the environment variable $ENV{PERL_CPM_MIRROR} so that it is possible to set a mirror without invoking cpm directly with --mirror.

A specific usecase for this:

You create a base docker image where you have PERL_CPM_MIRROR set to a sensible default value (other than metacpan.org). Containers based on this image picks that up automatically, so that a developer just running cpm install ... in their Dockerfile will get packages pulled from the correct repository. This is especially useful in environments where you have many images that are based on one base image and you would like to avoid duplication of urls. If the url needs to change you can do that once and not have to go through all Dockerfiles and change them like you would have to if everyone was forced to specify --mirror.

This change is however not limited to the above usecase. It also makes it convenient to do a quick export PERL_CPM_MIRROR=... in your environment and from that point on not have to remember to run cpm with --mirror all the time.

I've made the change in such a way that --mirror always has precedence if specified on the commandline, with a fallback to $ENV[PERL_CPM_MIRROR if set, or lastly https://cpan.metacpan.org as a last resort.

@christopherraa
Copy link
Author

Oh, and the tests run fine locally by the way, but your approval @skaji is required to get the Github actions to run.

@skaji
Copy link
Owner

skaji commented Aug 4, 2021

Thanks for the PR.
I have a plan to deprecate --mirror option, and to introduce --source option.

@christopherraa
Copy link
Author

Will --source also be overridable from $ENV? Also do you have an approximate timeline for when --mirror would be deprecated and --source introduced?

@skaji
Copy link
Owner

skaji commented Aug 4, 2021

I'm not sure yet.

@christopherraa
Copy link
Author

Would you be open to accepting this pull request until such time as there is a set timeline for --source? Having this feature would simplify things a great deal.

@skaji
Copy link
Owner

skaji commented Aug 4, 2021

Just out of curiosity, why do you want to change mirror?

@christopherraa
Copy link
Author

christopherraa commented Aug 4, 2021

Two main reasons:

  • The default metacpan mirror is very slow compared to what we can get having a local mirror. Orders of magnitude slower.
  • It is convenient to have our internal packages in the same mirror. These are packages that cannot be released to cpan.

@skaji
Copy link
Owner

skaji commented Aug 4, 2021

OK, thanks.

It is convenient to have our internal packages in the same mirror. These are packages that cannot be released to cpan.

Then I recommend you to use --resolver, not --mirror.

@christopherraa
Copy link
Author

Does resolver support being set through an environment variable? I can't seem to see any trace of this in the current code.

The main purpose of this pull request is being able to not specify mirror / resolver on every invocation of cpm. This is especially useful in the scenarios I described initially in this pull request.

For cpanm we have PERL_CPANM_OPT wherein one can specify all options, but I must say I prefer having a greater granularity and that is why I propose using the variable the way I do. I also feel this (using PERL_CPM_MIRROR as a variable) is more in line with your previous use of overriding through environment variables (eg. PERL_CPM_PREBUILT).

To reiterate:

  • I am not trying to change resolving with this pull request. Resolving is already working just fine.
  • The only issue I am trying to address is to be able to set which mirror to use through the environment

But to change things around: what is your main concern in accepting support for this? And is there anything I can change in the pull request so that your concerns are addressed?

@christopherraa
Copy link
Author

Just wanted to follow up on this. Is there no interest in being able to set parameters through environment variables like this merge request enables?

@skaji
Copy link
Owner

skaji commented Aug 19, 2021

I have a plan to deprecate --mirror option, so I don't want to add a new feature around mirror.

@christopherraa
Copy link
Author

I have a plan to deprecate --mirror option, so I don't want to add a new feature around mirror.

Are you open to getting support for letting resolvers be set by env-variables? If so I am more than happy to change this merge request so that mirror is not changed but resolvers get the added functionality.

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