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

Support git repos / http folders in packages stanza #199

Closed
mboes opened this Issue Jun 6, 2015 · 21 comments

Comments

Projects
None yet
8 participants
@mboes
Contributor

mboes commented Jun 6, 2015

Currently, if a package depends on a patched up upstream, the recommendation found in the FAQ is to check out the package as a git submodule, or unpack the source code for that package in some local directory.

However, often it's not so much that a user wants to depend on a patched up package, as the fact that the user is depending on an unreleased version of upstream. In this case it would be nice to be able to directly point to a remote repository, or to a commit hash in some remote repository, directly in the stack.yml file without needing to checkout the remote code just for the purpose of building against it.

@snoyberg snoyberg added this to the Later improvements milestone Jun 6, 2015

@feuerbach

This comment has been minimized.

Contributor

feuerbach commented Jun 7, 2015

Wow, that'd be pretty awesome!

@chrisdone

This comment has been minimized.

Member

chrisdone commented Jun 7, 2015

+1 I've had the use-case once or twice.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

If we make some simplifying assumptions, this should be trivial. In particular: assume that the URL or Git commit represents something immutable. Then we unpack/clone into a directory inside .stack-work and just treat it as an unwanted local package. Does this sound like a reasonable simplification?

@snoyberg snoyberg modified the milestones: First stable release (0.1.0.0?), Later improvements Jun 9, 2015

@chrisdone

This comment has been minimized.

Member

chrisdone commented Jun 9, 2015

Where "unwanted" means "not a wanted target" in the Make-like stack build step of demanding stack build TARGET1 TARGET2 or when stack build the targets are the implicitly wanted packages listed in the stack.yaml file.

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Jun 9, 2015

cabal-meta does much of this and works with darcs also

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

I already have an issue open about merging in mega-sdist functionality. Are you opposed to absorbing a few other of the cabal-wrapping tools we've created over the years?

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Jun 9, 2015

This is another tool that could possibly stay as a separate executable, except that the tool could benefit from stack's knowledge of stack.yaml. The downloading tool could look for a stack.yaml in the repo (like how cabal-meta looks for a sources.txt) to find where the package is in the repo instead of requiring the user to supply that information (default of course would be ./).

@snoyberg snoyberg self-assigned this Jun 9, 2015

@snoyberg snoyberg added ready and removed ready labels Jun 9, 2015

@dbp

This comment has been minimized.

dbp commented Jun 9, 2015

This is great. The only thing I'd add is that some (reasonably prominent) haskell packages put more than one package in the same repository (as subdirectories). For example, all of the digestive-functors-* family of packages exist within one git repository. In the tool that I wrote (that I'd love to replace with stack), the grammar I defined for these kind of dependencies is:

repo-path:git-ref+subdir1,subdir2, where the git-ref is optional (if missing, defaults to master), and the subdirs are also optional, but when present, are the path to the actual package(s). When missing, it's assumed that the package is at the root. (Note that the comma separated shorthand is unecessary, as you could just have multiple entries, which may be better for stack).

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

Despite my better judgement, I decided to take a crack at this right now, so your timing is perfect @dbp. I was just thinking about what the syntax would be, and of the subdirs.

Another option I was toying with was, instead of trying to shoe-horn everything into a single string, turning it into a YAML object, e.g.:

projects:
- some/dir
- location: another/dir
- location: https://github.com/foo/bar
  commit: deadbeef
  subdirs:
  - subdir1
  - subdir2

It feels kind of wordy to me, but also may be much easier to work with long term. It also allows us an easy way to encode extra useful information, like "never run test suites on this package" (which is needed for #219).

We can bikeshed the syntax easily later, the meat of this will be the rest of the implementation. I'll start on a branch that uses the YAML syntax, and we can figure out what the real thing will look like later.

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Jun 9, 2015

I was just going to chime in and suggest favoring YAML over grammar.

@dbp

This comment has been minimized.

dbp commented Jun 9, 2015

I think what @snoyberg proposed actually looks pretty good. At least for me, most of the time that this comes up are when I find a bug (or needed improvement :P) in a released package and need to patch it until the fix is released upstream, so having it be wordy really doesn't matter (as there are rarely many).

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

I've implemented the stack-specific stuff already, and now just have some stubs for downloading/unpacking/cloning at:

038ec35#diff-6cd1b827a90e053dc1b700465b1a94c0R316

I slightly tweaked the syntax above regarding Git locations to start with the word git to make it easy to disambiguate from HTTP and local directories, as well as make it easy to add support for other version control systems in the future. An example from WAI is:

- location: git https://github.com/yesodweb/wai master
  subdirs:
  - wai
  valid-wanted: false

Obviously using master like that is a bad idea since it's not immutable.

snoyberg added a commit that referenced this issue Jun 9, 2015

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

Alright, I've implemented HTTP support. Git support should be much the same, I'm just too tired to continue. If someone wants to take a crack at it, have fun.

snoyberg added a commit that referenced this issue Jun 9, 2015

@snoyberg snoyberg assigned mboes and unassigned snoyberg Jun 9, 2015

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 9, 2015

Alright, it's implemented :)

@mboes Want to give it a test drive?

@mboes

This comment has been minimized.

Contributor

mboes commented Jun 9, 2015

This looks awesome. Thanks a bunch!

Now go to sleep. ;)

@mboes

This comment has been minimized.

Contributor

mboes commented Jun 9, 2015

@snoyberg leaving open - close once branch is merged.

@mboes mboes removed the need confirmation label Jun 9, 2015

@DanBurton

This comment has been minimized.

Contributor

DanBurton commented Jun 10, 2015

Just wanted to leave my +1 on this feature. Love it.

@gregwebs

This comment has been minimized.

Contributor

gregwebs commented Jun 10, 2015

I would go farther with the YAML and not require any custom parsing.

For reference, here is Ruby's Bundler. It uses Ruby's hash syntax, which is conceptually like YAML.

gem 'nokogiri', :git => 'https://github.com/tenderlove/nokogiri.git', :branch => '1.4'

With stack, we could have

{git: "https://github.com/tenderlove/nokogiri.git", commit: "1.4"}

This does not necessarily preclude allowing the custom parsing for a location field if that is still desired for some reason. But it makes it easier to document the available configurations and gets us out of the business of parsing.

@3noch

This comment has been minimized.

Member

3noch commented Jun 10, 2015

+1 on less custom parsing

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 10, 2015

I like the idea of supporting both a short syntax and a more complete syntax like you've demonstrated here. Would you be able to add a commit to extend the FromJSON instance?

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jun 10, 2015

OK, merged to master, closing.

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