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

Add development tools recommendation in opam file (variable) and cli #5016

Merged
merged 5 commits into from May 20, 2022

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Jan 21, 2022

At the moment it's not finished, it needs a proper look at the modification (it's just seddish adds)
fix #4959
@rjbou edit:
All clear! Renamed to with-tools : available in cli opam install pkg --with-tools and in opam files as a variable depends: "foo" {with-tools}. Have the same behaviour than with-test and with-doc.

@rjbou rjbou added the PR: WIP Not for merge at this stage label Jan 21, 2022
@rjbou rjbou added this to the 2.2.0~alpha milestone Jan 21, 2022
@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Jan 21, 2022
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!! ❤️

src/client/opamLockCommand.ml Outdated Show resolved Hide resolved
doc/pages/Manual.md Outdated Show resolved Hide resolved
@rjbou
Copy link
Collaborator Author

rjbou commented Jan 21, 2022

No, it's not yet good, i set it in a commit, but not in the PR message itself, it's definitively a wip, i just made a sed ftm.

src/client/opamLockCommand.ml Outdated Show resolved Hide resolved
@AltGr
Copy link
Member

AltGr commented Feb 7, 2022

How does this interact with the previous behaviour ? E.g. are dev deps still turned on automatically when the package doesn't have a fixed checksum ? If so do we have a --without-dev to counter it ? The manpage should be clearer about this.

@rjbou
Copy link
Collaborator Author

rjbou commented Feb 9, 2022

It doesn't replace dev, and ye i agree the doc need to be quite clear about this new option/variable. with-dev is meant for development environment, and do not interact at all with dev/--dev variables/option. see referenced issue for more complete view of the needs.

@dra27
Copy link
Member

dra27 commented May 17, 2022

From discussions last December, we proposed --with-tools

@rjbou rjbou moved this from PR in progress to PR to review in Opam 2.2.0 May 18, 2022
@rjbou rjbou removed the PR: WIP Not for merge at this stage label May 19, 2022
@rjbou rjbou changed the title Add `--with-dev' option Add development tools recommendation in opam file (variable) and cli May 19, 2022
@rjbou
Copy link
Collaborator Author

rjbou commented May 19, 2022

Updated!

@rjbou rjbou moved this from PR to review to PR finalised (merge with CI) in Opam 2.2.0 May 20, 2022
@rjbou rjbou merged commit 2571522 into ocaml:master May 20, 2022
Opam 2.2.0 automation moved this from PR finalised (merge with CI) to Done May 20, 2022
@rjbou rjbou moved this from Done to To do in Opam 2.2.0 May 20, 2022
@dbuenzli
Copy link
Contributor

dbuenzli commented Jun 1, 2022

I guess I'm a bit late to the party but I think --with-tools is a pretty confusing name. Packages do install tools or not (I have many packages that optionally install command line tools if cmdliner is installed). I can already see people trying to install them using this command line switch. Maybe --with-dev-tools ?

(Personally I'm not entirely convinced by the actual need for the initial request, I'm not sure what's wrong with tagging these dependencies with dev, it looks like micro management of dependencies)

@dbuenzli
Copy link
Contributor

dbuenzli commented Jun 1, 2022

In fact I think for-dev would be a much better and less confusing name.

@kit-ty-kate kit-ty-kate added this to PR in progress in Opam 2.2.0 via automation Jul 11, 2022
@kit-ty-kate kit-ty-kate moved this from PR in progress to Done in Opam 2.2.0 Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

Allow to define dependencies as "development only", regardless how the package is built
5 participants