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

Installing dependencies without "test" phase #651

Open
dboehmer opened this issue Oct 7, 2022 · 7 comments
Open

Installing dependencies without "test" phase #651

dboehmer opened this issue Oct 7, 2022 · 7 comments

Comments

@dboehmer
Copy link
Contributor

dboehmer commented Oct 7, 2022

I’m going to build a minimal Docker image for an application that should only include runtime dependencies. I want to install all dependencies with --notest and just realized that this still installs all dependencies of my app’s cpanfile’s test phase.

I suggest adding options --without-test and --with-test just like the new --with-configure.

cpanm --installdeps --without-test .

Or is there any other way to do this?

@ehuelsmann
Copy link

I'm having the same issue. +1 for this proposal (or a documented alternative).

@dboehmer
Copy link
Contributor Author

dboehmer commented Aug 25, 2023

TL;DR: cpanm currently implements 5 use cases of skipping/running tests that I found. The CI use case proposed above is not yet implemented when installing from a cpanfile. Just defining how it should work isn’t easy because the whole logic is complex and confusing. I propose 3 solutions and ask for feedback.

I was working on a PR with a proof of concept for the proposed --without-test and --with-test but now I am confused about the actual meaning of --notest. I tried to look it up, fell into a rabbit hole and spent a few hours on that! 🎩

This code selects the phases that shall be installed:
https://github.com/miyagawa/cpanminus/blob/287cfbda837ab74160a068a4ce21c98b8a0c5b44/Menlo-Legacy/lib/Menlo/CLI/Compat.pm#L1982-1986

In my branch I added a commit trying to make this more readable: dboehmer@108981e

So there’s already logic to skip the test phase dependencies! 🎉 The test phase is only skipped if:

  • --notest was set—reasonable 👍
  • and depsonly() returns false—this is where it gets quite complex (see code) 😕

The man page https://github.com/miyagawa/cpanminus/blob/287cfbda837ab74160a068a4ce21c98b8a0c5b44/App-cpanminus/script/cpanm.PL#L136-143 on --notest does not indicate that complexity but seems pretty clear. 👌

-n, --notest

Skip the testing of modules. Use this only when you just want to save
time for installing hundreds of distributions to the same perl and
architecture you've already tested to make sure it builds fine.

Defaults to false, and you can say --no-notest to override when it
is set in the default options in PERL_CPANM_OPT.

I tried to sum up the actual conditionals but there are too many levels of negation that I couldn’t wrap my head around. Instead I made a truth table of the 5 inputs with all 32 combinations:
Screenshot_20230825_143826

After all there are only 3 cases where test phase is skipped. --notest must always be given what is reasonable. I reduced the truth table to these four relevant cases:
Screenshot_20230825_144005

Status quo:

  • case 17/18: When installing a dist with --notest all tests and test dependencies are skipped—reasonable 👍
  • case 19/20: When installing dependencies from a cpanfile with --installdeps --notest only the test dependencies of the dist’s dependencies are skipped, but the cpanfile’s test dependencies get installed—this might be someone’s use case

Question @miyagawa: Why are all test dependencies considered (regardless of --notest) if --scandeps or --showdeps is active? I think both would work fine with test dependencies skipped.

I’d like to see the code revisited because I found the complexity of the logic and the naming of states like depsonly() confusing and overwhelming. Questions I tried to answer myself:

  1. Can we improve or fix the logic of selecting the test phase?
  2. Do we need additional CLI options to serve all use cases?

To find out I made another truth table over installing/running primary/secondary test dependencies from dist/cpanfile with again 32 cases. But I could rule out most of them—e.g. not installing test depenencies but running tests is invalid, installing secondary test dependencies but not running the tests is pointless. I found 5 actual use cases:

Use case primary test dependencies and tests secondary test dependencies and tests from cpanfile from dist
1. fast install/lightweight container (original request in this issue) skip skip currently impossible --notest 👍
2. fast custom CI (quickly install dependencies, test run later by CI script) install skip --installdeps --notest 👍 --installdeps --notest 👍
3. fast dist CI (quickly test dist built by previous CI step) install & run skip (runnings tests requires dist) cpanm --installdeps --notest X && cpanm X
4. developer (wants to run tests manually) install install & run --installdeps 👍 --installdeps 👍
5. safe install install & run install & run (runnings tests requires dist) default 👍

I see three possible solutions to provide the missing use case 1 fast install/lightweight container from cpanfile:

  • solution A: implement CLI option --without-test which overrides selection of phases from the primary cpanfile/dist
    • cons: 1 more CLI option, possible confusion between --notest and --without-test, requires new documentation and implementation, possible side effects with other options
    • pros: no breaking changes
  • solution B: change --installdeps --notest to also skip the test phase from cpanfile—but this breaks use case 2 fast custom CI from cpanfile: Then how to quickly install test dependencies without their tests? Do we need --with-test to fix that? A different idea: cpanm --showdeps . | cpanm --notest as long as --showdeps lists all dependencies including the test phase …
    • cons: breaking changes, someone’s CI might break
    • pros: current logic of --notest is undocumented
  • solution C: change --showdeps to adhere to --notest and not list test dependencies. Then this should work:
    cpanm --showdeps --notest . | cpanm --notest?

@miyagawa: What are your thoughts about this? Do you understand my reasoning? Which solution would you consider?

@dboehmer
Copy link
Contributor Author

@ehuelsmann Can you package your code in a CPAN dist file? Then you could use cpanm --notest MyApp.tar.gz and it would install with any test dependencies.

@miyagawa
Copy link
Owner

You're making the behavior look more complex than it actually is :) The logic to skip the testing deps is as simple as (as you put it somewhere in the middle): "test deps are skipped if --notest is specified, except for test deps in the main target when running with --installdeps.

the reasoning for this is that cpanm --installdeps . is considered a special mode for "installing dependencies of the application/module in the current directory" and doing this in CI is one of the main use cases, i.e. you might not want to run tests of the deps, but after installing the deps, you'll run its own tests.

Why are all test dependencies considered (regardless of --notest) if --scandeps or --showdeps is active?

These two options are deprecated and the behavior should be considered accidental.

I think --with-test makes some sense, given we already have --with-develop and --with-configure which works the same way.

@ehuelsmann
Copy link

ehuelsmann commented Aug 25, 2023

the reasoning for this is that cpanm --installdeps . is considered a special mode for "installing dependencies of the application/module in the current directory" and doing this in CI is one of the main use cases

Although I agree you'd want this in CI scenarios, I think that --without-test makes sense when the application in the current directory is actually being installed into a container and that the test dependencies are not desired for the container use-case. There is no need for --with-test since that's already the default mode of operation for "the application in the current directory" (where I'm missing the means to disable that).

@miyagawa
Copy link
Owner

miyagawa commented Aug 25, 2023

There is no need for --with-test since that's already the default mode of operation for "the application in the current directory" (where I'm missing the means to disable that).

You'll need it in case --without-test is specified in the PERL_CPANM_OPT for symmetry.

@dboehmer
Copy link
Contributor Author

Ok, my PR is adding --with-test and --without-test is half done. I’ll later upload what I have. I’ll probably need some help getting the tests right and identifying potential problems with other options.

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

No branches or pull requests

3 participants