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

shards install #81

Closed
sdogruyol opened this Issue Nov 22, 2015 · 19 comments

Comments

Projects
None yet
9 participants
@sdogruyol
Member

sdogruyol commented Nov 22, 2015

Much like go install github.com/x/y or cargo install --git 'github.com/x/y'

I think this eases the workflow of cloning the project, manually building and then exporting to path.

What do you think?

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented Nov 22, 2015

I'm not a fan of this. I prefer to have a shard.yml with a lock that's always the source (like Bundler) than "manually" fiddling with the libs folder.

@asterite

This comment has been minimized.

Contributor

asterite commented Nov 22, 2015

Not a fan either. The dependency should be present in a committed file so the build is reproducible.

@sdogruyol

This comment has been minimized.

Member

sdogruyol commented Nov 22, 2015

How about the new cargo installin Rust? They also have cargo.toml for reproducible builds just like bundler.

More at rust-lang/rfcs#1200

@ysbaddaden ysbaddaden added the question label Jan 4, 2016

@f

This comment has been minimized.

f commented Mar 8, 2016

Something like shards install --git sdogruyol/kemal@master would update the shard.yml file to make the installation easier. And so it will still have a shard.yml with a lock file which won't change anything but we have an installation helper.

@marceloboeira

This comment has been minimized.

marceloboeira commented Mar 16, 2016

@f I like this approach, but I would suggest something less verbose, just:

shards install $user/$repo@${$tag, branch, commit}

@spalladino

This comment has been minimized.

Member

spalladino commented May 26, 2016

Sounds similar to what npm does when the --save flag is specified, and I can't remember a time when I did not use it.

@tommoor

This comment has been minimized.

tommoor commented May 29, 2016

Hey! So I've just been intro'd to the Crystal language last week and I thought that this functionality needed to exist too so I started taking a stab at adding it in 😄 You can see my progress here (master...tommoor:feature/install-shard), right now the shard installs with the following syntax:

shards install user/repo@version

I'd like to have the command update the shard.yml npm style but I can't decide on the best approach for that - any thoughts?

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented May 29, 2016

Thanks for spending time on this, and dissecting shards internals!

Write shard.yml

This is a problem: Bower and NPM always reformat the JSON file. I hate that a script changes my formatting or reorders my file as it pleases. Outputting YAML is also trickier than JSON.

Supporting resolvers

Shards isn't GitHub centric and doesn't have to be Git centric at all. user/repo can refer to GitHub, GitLab or Bitbucket or anything else. We'd then need resolver:user/repo and have as many methods in the install command, but it shouldn't have to know anything about resolvers. Maybe delegate to each resolver? but resolvers are already complex enough. And what about the base Git resolver?

I don't think such a tiny helper for adding a dependency every once in a while is worth all that complexity.

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented May 29, 2016

To be completely honest: I never use this feature in any package manager and usually think it's more harmful than useful, and creates unrequited complexity.

This is my sole opinion on this. If I used this feature I would happily implement and maintain it. The problem is that I don't, and I don't want to support and maintain this feature...

@tommoor

This comment has been minimized.

tommoor commented May 29, 2016

I hope you won't let your personal usage patterns get in the way of a potentially useful feature for the rest of the community... 😄

I'm absolutely happy to build it in such a way that resolvers are supported without adding complexity to the resolver classes themselves and it won't rearrange your yml file (although personally, I don't think that the yml file contents should be in the same order in every project!).

A couple of good reasons to include this that I can see:

  • Pretty much every other package manager has this, it's expected functionality by newcomers and this aligns strongly with Crystals goal of being easy to use (even in this thread there is a lot of support).
  • It makes installation instructions simpler and verbally communicable (Just look at pretty much any npm module's README)
  • It takes something manual and makes it scriptable (which hacker doesn't love that?)

I won't work on this any further until you let me know either way.

@asterite

This comment has been minimized.

Contributor

asterite commented May 29, 2016

Just to be sure, is this about installing binary executables to be available in the PATH, or just to install a library in the libs directory?

@tommoor

This comment has been minimized.

tommoor commented May 29, 2016

@asterite this is following on from the last three comments before mine. It's about installing a shard with a command that will automatically update the shard.yml the same as npm install blah --save

@asterite

This comment has been minimized.

Contributor

asterite commented May 29, 2016

I see. I agree more with @ysbaddaden, mostly because installing a shard is just copy/paste from the shard's instructions. It's also hard to do because we'd need to parse the yaml, modify it, and output it again, preserving comments and structure. I think this is very hard to do. And then there's the resolvers problem.

But if someone manages to implement this correctly, I don't think it's harmful. It's just that it will also involve a copy paste, only to the terminal instead of to a text file.

@tommoor

This comment has been minimized.

tommoor commented May 29, 2016

That's disappointing, I think it's important that the project include these affordances and improve the quality of the tooling if you want to appeal to folks like JavaScript and Ruby developers.

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented May 29, 2016

  • NPM & Bower (JS) indeed have this helper;
  • Bundler (Ruby) only installs gems manually specified in Gemfile;
  • Gem (Ruby) deals with distributing software and installs them in a centralized directory (no dependencies file until recently);
  • Go: installs and builds dependencies... but doesn't provide a dependencies file (or does it?);
  • Cargo (Rust): installs binaries into PATH (and doesn't touch Cargo.toml), but third-party software deal with this (eg: cargo-edit).

I emphasize what @asterite said: if somebody implements this perfectly, in a simple yet abstract way, and with proper YAML handling, we'll consider it. I just don't think this is worth the hassle —unlike #1 (conflict resolver).

@pfertyk

This comment has been minimized.

pfertyk commented Nov 19, 2016

I'm new to Crystal and recently I tried to add a dependency to my program. It was called stumpy_png, and I found it on http://crystalshards.xyz. As a new user I was pretty sure that running shards install stumpy_png would install the dependency in the libs directory. After a while I found out that shards install installs the dependencies from shard.yml file, so I started looking for a command like shards add stumpy_png that would update the shard.yml with the proper info about my new dependency. But there seems to be no command like that. I guess the problem here might be the lack of a central shards repository (crystalshards.xyz seems to just list them and redirect to github).

As a user I would really appreciate a command like shards add <dependency_name> that would update shard.yml file for me. NPM, pip and probably many other package managers have this feature. It there a way to implement it using only what crystalshards.xyz provides at the moment? If not, is there a plan to create a centralized shards repo?

If that is not possible, I guess the command would have to include the resolver, username and repo (like @ysbaddaden said):

shards add github:l3kn/stumpy_png

That is only slightly easier than modifying the shard.yml file manually (since I have to know the provider and username, not just the name of the dependency).

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented Nov 19, 2016

Closing as "won't implement". I don't want to lead people on believing this may make it.

I apologize if you'd really love this feature, but Shards requires that you manually, and carefully edit your shard.yml, then type shards install or shards update depending on the action you want. There will be no CLI arguments or options that will mess your shard.yml (it's read-only).

If you really can't live without that feature, please create a shards-edit utility that will edit your shard.yml then delegate to shards to install dependencies.

@pfertyk Shards should fail when unknown arguments are left on the command line, so it would be obvious that you can't install a dependency that way. Along with proper documentation of commands (which is lacking right now).

@ysbaddaden ysbaddaden closed this Nov 19, 2016

@ysbaddaden

This comment has been minimized.

Member

ysbaddaden commented Nov 19, 2016

Note that the initial idea of this thread, that is cloning, building and installing an app directly from Git is interesting. It could help distribute developer software, without having to care about system packages (APT, APK, PKG, Homebrew, etc). Yet, I don't think it will make it anytime soon, and maybe a generic solution (one not tied to a single language) would be better.

@sam0x17

This comment has been minimized.

sam0x17 commented Jul 27, 2018

You don't technically have to rewrite the YAML file. I would suggest just creating the dependencies section if it doesn't already exist (using string operations), and appending to the end of the dependencies list (don't bother sorting). This will preserve comments but get that nice npm install ---save [package name] syntax that everyone wants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment