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

Fetch with dependencies #1898

Closed
wants to merge 2 commits into from
Closed

Conversation

Panke
Copy link
Contributor

@Panke Panke commented Mar 14, 2020

No description provided.

@dlang-bot
Copy link
Collaborator

Thanks for your pull request and interest in making D better, @Panke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

@Panke
Copy link
Contributor Author

Panke commented Mar 14, 2020

I am needing this or something similar for bazel support (#1889)

@Geod24
Copy link
Member

Geod24 commented Mar 15, 2020

Needs a rebase. And for the name, --recursive is more common (e.g. it's what git and many CLI tools use).

@Panke
Copy link
Contributor Author

Panke commented Mar 16, 2020

Needs a rebase. And for the name, --recursive is more common (e.g. it's what git and many CLI tools use).

--recursive was my first thought, but we got some use of recursive that is quite dissimilar to this one, like #1873 which means sub packages and not dependencies.

I couldn't care less for the name though. If no one else chimes in, it will be recursive.

@Geod24
Copy link
Member

Geod24 commented Mar 17, 2020

I think for dub fetch, --recursive is pretty unambiguous. You cannot fetch sub-packages independently.

@Geod24
Copy link
Member

Geod24 commented Mar 17, 2020

Also need a release notes entry, this is a change that might be interesting to a few people (e.g. in build scripts).

@WebFreak001
Copy link
Member

WebFreak001 commented Mar 17, 2020

should we make this a switch? imo this should be the default

workflow for someone without internet:

go to internet cafe, connect to internet
fetch all required dependencies
disconnect from internet, go home
build stuff locally -- oops, dependencies of fetched dependencies aren't there by default

@Geod24
Copy link
Member

Geod24 commented Mar 17, 2020

We can't have self-reference / circular reference, can we ? That would be one argument against it.
Having it the default for dub fetch makes sense. IDK about --cache=local though.

@WebFreak001
Copy link
Member

definitely needs a test for recursive dependencies here then

@Panke
Copy link
Contributor Author

Panke commented Mar 17, 2020

We can't have self-reference / circular reference, can we ? That would be one argument against it.
Having it the default for dub fetch makes sense. IDK about --cache=local though.

I don't think --cache=local should be a default. When --recursive becomes the default, what do we call the switch? --shallow? --not-recursive --single --no-dependencies

@Geod24
Copy link
Member

Geod24 commented Mar 18, 2020

I don't think --cache=local should be a default.

Oh I phrased my message very poorly. I meant "IDK if having it the default for cache local makes as much sense". But after reviewing again, I think it does too.

what do we call the switch?

I'd say, in doubt, do what git does. So --shallow.

@Panke
Copy link
Contributor Author

Panke commented Mar 18, 2020

What we could do is: download everything that is not in the --cache=here. So that afterwards everything is available 'here'

Building a package that was locally cached failed, because
the dependencies whould be fetched at the wrong destination.

For example if one tries to build the package 'optional':

$ dub --cache=local fetch optional
-> optional is now at ./.dub/package/optional
$ dub --cache=local build optional
-> fails because the dependency bolts was fetched to
./.dub/package/optional/.dub/package/bolts.

This is fixed by not overwriting the root path if dub builds
a package from the cache.
Dependency resolution and retrieval is copied from the
dub build command.
@Panke Panke force-pushed the fetch-with-dependencies branch from 7da1eaa to 55ab80a Compare March 20, 2020 17:08
@Panke
Copy link
Contributor Author

Panke commented Mar 20, 2020

This turns out to be more annoying than I thought. This works only for dependencies that are dependencies of the default configuration, but won't fetch anything for e.g. vibe-d.

I could use some pointers on how to get a list of transitive dependencies.

@WebFreak001
Copy link
Member

try checking the selections and/or generating them for the fetched package, they are recursive

@andre2007
Copy link
Contributor

When I execute following commands:

dub fetch vibe-d
dub describe vibe-d

vibe-d and all dependencies are downloaded. Therefore dub describe vibe-d does the fetch with dependencies. Maybe you could check how it works here and reuse the already available functionality?

@andre2007
Copy link
Contributor

Related #1930

@Geod24
Copy link
Member

Geod24 commented Jan 12, 2024

As far as I can tell this was done by #2786

@Geod24 Geod24 closed this Jan 12, 2024
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.

5 participants