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

dub select, dub deselect, dub upgrade -l #2359

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WebFreak001
Copy link
Member

@WebFreak001 WebFreak001 commented Aug 3, 2022

first part to interact with dub.selections.json

dub explain comes in a separate PR. (for issues see #1064)

fix #123

@PetarKirov

This comment was marked as outdated.

@WebFreak001

This comment was marked as outdated.

@maxhaton

This comment was marked as outdated.

source/dub/commandline.d Outdated Show resolved Hide resolved
source/dub/commandline.d Outdated Show resolved Hide resolved
source/dub/commandline.d Outdated Show resolved Hide resolved
@WebFreak001
Copy link
Member Author

WebFreak001 commented Aug 8, 2022

maybe it's better to separate dub select and dub explain into two PRs.

EDIT: done, this PR is now only dub select

@maxhaton

This comment was marked as outdated.

@Geod24

This comment was marked as outdated.

@WebFreak001 WebFreak001 force-pushed the dub-select branch 2 times, most recently from e3915d3 to 9e6e9c6 Compare February 13, 2023 00:53
@WebFreak001 WebFreak001 changed the title dub select + dub explain dub select, dub deselect, dub upgrade -l Feb 13, 2023
@WebFreak001 WebFreak001 marked this pull request as ready for review February 13, 2023 00:54
@WebFreak001
Copy link
Member Author

I moved the dub explain part out of here into a separate branch for now, as the select interface is quite a lot easier to add, show and test.

dub upgrade -l now shows this kind of output and does an implicit dry run: (can be made a non-dry run)

image

dub select pkg ver can be used to insert or change a dependency in dub.selections.json

dub deselect pkg can be used to remove a dependency from dub.selections.json

@WebFreak001
Copy link
Member Author

test failure seems unrelated? (dmd-master only)

@WebFreak001
Copy link
Member Author

need an approve to merge @Geod24

@Geod24
Copy link
Member

Geod24 commented Mar 2, 2023

I'm a bit confused, I thought this was only for optional dependencies, originally, but there's no mention of it ?

combined with `--missing-only` (now available as `-m` as well) to show what is
being done internally when running `dub build`. Both an upgrade as well as
listing can be done by running `dub upgrade --list --dry-run=false` or
`dub upgrade -l -dfalse`.
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure -dfalse works ? That would be odd - and not something we should recommend. Or perhaps you mean -d false ?

packages + packages to upgrade in an easy-to-read interface. This can be
combined with `--missing-only` (now available as `-m` as well) to show what is
being done internally when running `dub build`. Both an upgrade as well as
listing can be done by running `dub upgrade --list --dry-run=false` or
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit bothered by the fact we now have more switches to do the same thing, slightly differently.
We have dub -v and dub upgrade -l and -l implies dry run but we can also use -d false...
How about simplifying it and always having dub upgrade -l behavior, and just keeping dry-run for dub upgrade -l ?
Note that dub upgrade -l is not IMO very consistent CLI behavior: options (things starting with dash) shouldn't be used to implement subcommands, they should only slightly modify command behavior. So if you want to keep the dub upgrade -l behavior I would have gone with dub upgrade list. I'm not attached to the specifics, so happy to hash this out.

"If one or more package names are specified, only those dependencies will be upgraded. Otherwise all direct and indirect dependencies of the root package will get upgraded."
"If one or more package names are specified, only those dependencies will be upgraded. Otherwise all direct and indirect dependencies of the root package will get upgraded.",
"",
"List output format: (not script-safe)",
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't write "(not script-safe)". Except for dub describe, nothing is script safe.

source/dub/commandline.d Show resolved Hide resolved
Comment on lines +228 to +229
/** Returns a modified dependency that's relative to a given path, if the
path was absolute.
Copy link
Member

Choose a reason for hiding this comment

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

I think we already have a function to do this no ?

/// Selects a certain version for a specific package.
void selectVersion(string package_id, Version version_)
/// Selects the exact dependency for a specific package.
void selectVersion(string package_id, Dependency d)
Copy link
Member

Choose a reason for hiding this comment

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

I was trying to remove any use of Dependency from the API because it's almost always wrong. Why re-introduce it?

@Geod24
Copy link
Member

Geod24 commented Jun 18, 2024

@WebFreak001 : Do you want to pursue this ? It would be a useful addition.

@WebFreak001
Copy link
Member Author

I think this would need a rewrite since the underlying architecture changed quite a lot, which I'm currently not able to work on. I'd leave this open for anyone who wants to tackle it though

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.

Implement a way to select optional dependencies
4 participants