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

"Native reviews" don't work well and prevent from using Elm in production #105

Closed
dimsmol opened this issue Nov 14, 2015 · 9 comments
Closed

Comments

@dimsmol
Copy link

dimsmol commented Nov 14, 2015

I totally understand the idea of having good native code in official package repository, but for now it just doesn't work.

Most of the issues in this repo are "Native review" issues, the oldest one waits for more than 8 months (!!!) and the newest are waiting for weeks just to get the label that they are indeed related to native reviews. Also, I don't think the review is thorough enough to give any guaranties that the code is really good.

Looking at comments I see that people are giving up and abandon their projects before they have a chance to get through the review. Elm is young and a lot of people are experimenting with it, but they cannot do it if an experiment waits for being reviewed for months. Also, no one will join and improve far from ideal but promising library if it anyway cannot be used easily because it stuck in the review process.

What is worse, review requirement prevents from using Elm in production. Production blocked by the review process is a nightmare. Of course we can use git submodules or something but it isn't any way better than not having package manager at all.

I think we shouldn't prevent the packages from getting into repository by requiring the review. The state of affairs shows clear enough that there are no resources for such an approach.

If you think the situation will get better in future (which I doubt), we can just use some kind of warning within the repository saying that the package is native and haven't reviewed yet. The other way could be to have completely separated experimental repository, but I think this way everyone will just eventually use it and abandon the official one.

@evancz
Copy link
Member

evancz commented Nov 14, 2015

I think pretty much everyone agrees with this assessment.

I have written a bit about my upcoming plans here and it is pretty clear that this is probably the most important thing to resolve for Elm to keep growing and becoming useful for more folks.

I want you to know that I think this issue is important and that I am going to be focusing on it primarily as soon as I can. I also want you to know that I work as hard and as quickly as is possible. No one is slouching around here, but to get a good solution that is going to keep Elm healthy takes time.

I am going to be flying back to SF tomorrow and getting 0.16 out the door in the following two days, so I need someone in @elm-lang/packages to continue this discussion in my place.

@dimsmol
Copy link
Author

dimsmol commented Nov 15, 2015

Thank you for quick response!
And thanks a lot for the great job you all are doing!

I'm currently thinking on extending elm-package functionality, this is what we need anyway to use Elm in commercial projects:

  • allow to use alternative (private) registries (besides package.elm-lang.org)
  • allow to get files from private git repositories and possibly from other sources (private registry itself, tarballs hosted somewhere, etc.)
  • possibly allow to use direct link to some package as a constraint (like npm does)
    • e.g. "dimsmol/vdom-to-html": "git+ssh://git@github.dimsmol/vdom-to-html.git#some_commitish"
    • it will make experimenting and hacking around much easier, but it conflicts with the idea of strictly versioned packages so much, that I'm not sure if it can be done in a reasonable way
  • make elm-package work in docker (workaround this issue)

While it won't fix an issue with the reviews, it will allow to move further without delay. One can experiment with updated version of elm-package without disrupting anything and without even waiting for changes to become the part of the official distribution.

I want also to mention that there is an issue with current versioning scheme. It doesn't allow to make beta releases available for early adopters while still distinguish them somehow in a constraint. For instance, I can have an agreement that odd version numbers are unstable, but there is no way to make a constraint to choose stable versions only. And there is no way to tag release as beta or something. Not sure how to solve it though.

evancz pushed a commit that referenced this issue Nov 17, 2015
Give clearer description to help folks who feel like OP in #105
understand what the current situation is without digging through issues
or opening new ones.
@evancz
Copy link
Member

evancz commented Nov 17, 2015

Thanks :)

I added a note about what is going on in b35c30f so hopefully the root concern is addressed as best as possible without actually making the fixes yet.

Otherwise, again, pretty much all the things you have mentioned are on my personal roadmap for this project. Elm has grown a lot since the last time I focused on elm-package so pretty much all the things you have listed have become much more important, especially as our users have shifted from hobbyists who wanted to publish any community packages to businesses who have private packages. Right now, companies are doing work arounds for this. I bet @rtfeldman can describe what they are doing at NoRedInk. Point is, all the things you say are things that are now quite important and high on my priority list!

I am going to close this because I don't want to track work on these topics in this thread. Gotta keep things focused and close aggressively or hard-to-follow issues will bury the clear and actionable ones.

If you are interested in making some contributions, I'd focus on the Docker one. I really have no clue what the deal is there and am not an expert on containerization at all. So that's one where there'd be no toe stepping and you'd probably have a competitive advantage over me :)

@evancz evancz closed this as completed Nov 17, 2015
@evancz
Copy link
Member

evancz commented Nov 17, 2015

Also, I think the idea of allowing links to git repos would address the concern about beta releases. If people want to test early, have them point at a tag. I don't think beta releases make total sense because you may do an experimental version that you want to show people that is MAJOR, but then decide it was all dumb and end up with a MINOR release.

@rtfeldman
Copy link
Member

Strongly agree about git repos being the best solution for beta releases.

I think "arbitrary Git URL" covers every missing case except "relative local file path" (e.g. if you have a project with a tests/elm-package.json that wants to bring in the root project while adding test-specific dependencies like elm-test and elm-check, being able to say something like "../": "1.0.0 <= v < 2.0.0" instead of manually duplicating the root project's dependency list inside the tests' elm-package.json like we do now).

@evancz
Copy link
Member

evancz commented Nov 17, 2015

I don't actually have a nice list of all these ideas, though I'm sure they have come up in various forms before.

Would you @dimsmol be up for writing up your ideas and @rtfeldman's in as clear a way as possible in a gist? I am imagining a brief description of each proposed thing in a gist in the same sort of brief format as elm-lang/elm-make#59 with minimal examples.

If you can, please share it here, and I'll add it to elm-lang/elm-make#59 in the "issue updates" section that is for tracking this sort of stuff. It's likely it'll all happen in one big batch.

@dimsmol
Copy link
Author

dimsmol commented Nov 18, 2015

extended dependency syntax

I agree that allowing links to git repos covers concern about beta releases.

Here I want to discuss how we want to specify such "link to something" dependencies (in future they can be not only links to git, but also links to tarballs, local dirs, etc.). There are at least two possible options here:

  • Use the link as a constraint, like this: "user/lib": "git://github.com/user/lib.git#commitish" (this is the way npm works)
  • Use the link as a name, like this: "git://github.com/user/lib.git#commitish": "1.0.0 <= v < 1.1.0"

While the second approach is something people are already trying to use, I think it isn't what we really want. There are some reasons against it:

  • It's very logical to use a link as a constraint, because it is actually a constraint saying "I want exactly the version I'm referring to"
  • Link destination is already an exact version, so having additional constraint on the version has no benefits besides of ability to double-check author's intentions what effectively means author should specify her intentions twice, which is tedious
  • We can have other dependencies in the graph referring the same library and if they will have different name we won't be able to detect versions conflict

The third reason is the most important one. Here is an example. Let's assume we have the package x/main which depends on libraries y/a and z/b, but y/a also depends on z/b. The author of x/main wants to experiment with beta version of z/b, so she updates the dependency to refer git://github.com/z/b.git#beta. But she is not aware that y/a also has a dependency on z/b, e.g. "z/b": "1.0.0 <= v < 1.1.0".

Deps graph may look like this:

x/main ---> y/a ---> z/b
     \---------------^

If x/main will have updated dependency as "z/b": "git://github.com/z/b.git#beta" we will immediately find the conflict, because there is no guaranty thatgit://github.com/z/b.git#beta required by x/main is compatible with 1.0.0 <= v < 1.1.0 required by y/b.

But if x/main's updated dependency will be "git://github.com/z/b.git#beta": "1.0.0 <= v < 1.1.0" there is no guaranty that it has any relation to z/b referred in y/a and we cannot find they are different versions of the same lib.

So, I propose to go the way npm goes and stick to the first option, namely "user/lib": "git://github.com/user/lib.git#commitish". Maybe we can invent some other approach, but the second option doesn't look good anyway.

dependency conflicts

In the case described above, we have the conflict. But we can fork y/a, update it's deps to use "z/b": "git://github.com/z/b.git#beta" and use github link for forked version of y/a in x/main. This will require a lot of actions, but anyway we can do it and then experiment with beta of z/b as we need.

But the deps graph can be more complicated. There can be a number of libraries depending on z/b or we can have something sitting very deep into dependency graph depending on z/b. In this case we will need to fork and patch a lot of libraries and it becomes unreasonable. We cannot ask all the authors of those libs to do something for us because we aren't even sure yet that beta version of z/b is what we really need.

Example of complicated graph:

x/main ---> y/a1 ---> y/a2 ---> y/a3 ---> z/b
     |----> z/k1 -------------------------^
     |----> z/k2 -------------------------|
     |----> z/k3 -------------------------|
     \------------------------------------/

In this graph we need to patch all the packages y/a1 - y/a3 and z/k1 - z/k3 to have a chance to try git://github.com/z/b.git#beta with x/main.

How does npm deal with this? With npm there will be no conflict at all, x/main can use one version of z/b and the other libs can use other versions at the same time. This is a way how npm works, but it causes very bad problems, so we cannot use it.

Instead, I propose to add an ability to force top-level constraints over all the constraints dependency packages may have. For instance, having "z/b": "force git://github.com/z/b.git#beta" could make all the dependency packages use git://github.com/z/b.git#beta regardless of their own constraints for z/b.

While this is a potentially dangerous feature, it can be very helpful for experiments and can be a savior if you need to adopt an urgent security fix or quickly workaround a bug in someone's package dependencies. Of course elm-package should output all kinds of warnings when dealing with this option and it would be better to prohibit packages with forced deps from getting into official repository.

caches and downloaded packages dirs

Currently, downloaded packages are saved in the package cache and elm-stuff directory under the paths like user/name/version. But what should be the path for a github link?

Surely it cannot be user/name/version because the package can still have no proper version assigned (as @evancz said you may think it will be major change but end up with minor improvement or vice versa). So we cannot guess the version of the beta package without potentially clashing with some future release version.

npm solves similar problem by translating links into directory names, so git://github.com/user/lib.git becomes something like git_github.com!user/lib (see details).

Going this way we could have path like user/lib/translated_url. Is this a good way to go?

getting package constraints

Currently elm-package gets deps for the package from the store, checks everything and only then starts to download files. For github links there will be no entry in the store, so the only way to get package deps is to download it's elm-package.json and parse it.

For git, however, there can be a problem with downloading a single file from a repository. For instance, there is no way to download a single file from github using git+ssh protocol (which is preferred because it allows to use ssh certificates transparently). The same problem is present for tarballs as well.

Will it be ok to download the whole package just to get it's dependencies list and probably fail with conflict right after that? I think for certain situations we just haven't any other way, but maybe there can be some other ideas?

dev dependencies

@rtfeldman, according to your proposal to use "../": "1.0.0 <= v < 2.0.0" dependencies for tests subfolder to indicate it should inherit all the dependencies of parent folder. I think it would be better to have syntax like "inherit": "../" for this, because there is no reason to have version constraint here (as explained above) and inherit keyword will more clearly indicate what's going on.

But it can be even better to adopt npm's idea of dev dependencies. Those are dependencies that are meant to be installed in dev environment which may need additional libs for tests, build process or something. This way you will just add everything you need for tests into dev dependencies of the main elm-package.json.

What do you think?

@rtfeldman
Copy link
Member

I think this discussion should move to its own issue. 😄

@dimsmol
Copy link
Author

dimsmol commented Nov 18, 2015

let's continue here

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