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

Implement handling of local packages in the package-set #96

Merged
merged 9 commits into from
Feb 5, 2019

Conversation

f-f
Copy link
Member

@f-f f-f commented Feb 3, 2019

Implements support for local packages, as described in #88 (comment) (use case: bower link)

Basically we now try to distinguish if "URL"s in the package-set are referring to remote repos, or local filesystem path. In the latter case instead of trying to clone them, we just include their sources in the build (if they are listed as a dependency ofc). This basically mirrors the behaviour of stack.

Practical example: let's say I find a bug in simple-json. I manage to fix it locally, but I also want to see if the fix works for my project.

Since simple-json is already in the package-set, I can add the local package to the overrides in packages.dhall:

...

let overrides = { simple-json = upstream.simple-json // { repo = "../purescript-simple-json" } }

...

Note that if I list-packages, it is correctly listed as a local package:

$ spago list-packages
...
signal                v10.1.0   Remote "https://github.com/bodil/purescript-signal.git"
sijidou               v0.1.0    Remote "https://github.com/justinwoo/purescript-sijidou.git"
simple-json           v4.4.0    Local "../purescript-simple-json"
simple-json-generics  v0.1.0    Remote "https://github.com/justinwoo/purescript-simple-json-generics.git"
smolder               v11.0.1   Remote "https://github.com/bodil/purescript-smolder.git"
...

After adding it to the dependencies in spago.dhall, if I do spago install, it will not download it, but only its dependencies:

$ spago install
Installing 42 dependencies.
...
Installing "distributive"
Installing "refs"
Installing "identity"
Skipping package "simple-json", using local path: "../purescript-simple-json"
Installing "control"
Installing "enums"
Installing "st"
...

@f-f f-f changed the title Fix #88: implement handling of local packages in the package-set Implement handling of local packages in the package-set Feb 3, 2019
@f-f f-f requested review from btrepp and justinwoo February 3, 2019 22:12
app/Spago/Spacchetti.hs Outdated Show resolved Hide resolved
app/Spago.hs Show resolved Hide resolved
Copy link
Contributor

@btrepp btrepp left a comment

Choose a reason for hiding this comment

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

UX on this is pretty good, though you still need to specify the upstream dependencies. Granted not everyone would run into this.

You still need to do something like

let additions = {
    tiled = 
        mkPackage 
            (../purescript-tiled/spago.dhall).dependencies
            "../purescript-tiled"
            "local"
    ,pixijs = 
        mkPackage
            (../purescript-pixijs/spago.dhall).dependencies
            "../purescript-pixijs"
            "local"
}

Which isn't bad (unfortunately I can't find a way of dynamically building that as a dhall function). So this works great for me

@justinwoo
Copy link
Contributor

You could use record update on those, right? It's just that you need to override the url section?

@f-f
Copy link
Member Author

f-f commented Feb 4, 2019

@justinwoo: note that his example has additions, not overrides, so you need to specify the dependencies somehow: either listing them or pulling them from a spago config if the package you're importing has one

@btrepp: I'll add this note to the README

@justinwoo
Copy link
Contributor

So in this case you'd want to have a new mkPackage function that can make local library package definitions? Where one of the args passed in would be either the path or the imported record?

@f-f
Copy link
Member Author

f-f commented Feb 4, 2019

@justinwoo could you make an example of how it would look like?

I though about having a mkLocalPackage that takes only two args (path and deps), but in the end it's not worth it, because a usecase could be that once you upload your package, you just swap the local package with the URL of the new git repo. After all I'm not even sure we need mkPackage, since it's just producing a record.

@justinwoo
Copy link
Contributor

mkLocalPackage would be useful enough. mkPackage is also fine overall since you know that's what you're making rather than just some random record that you hope is fine, and also since you want to ensure that records are of type Package.

@f-f f-f merged commit 74e6fac into master Feb 5, 2019
@f-f f-f deleted the local-packages branch February 7, 2019 23:10
elliotdavies pushed a commit to elliotdavies/spago that referenced this pull request Jul 1, 2019
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.

3 participants