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

Support publishing to file repositories specified in ~/.sbt/repositories. Fixes #1570 #1579

Merged
merged 1 commit into from Sep 8, 2014

Conversation

copumpkin
Copy link
Contributor

This simply looks at the protocol specified in the repositories file and if it's a file URL, it makes a FileSystemResolver. There's some minor trickery involved in converting URLs to File objects when Windows UNC paths are involved. Once we get to assume Java 7, the Paths module should deal with that for us, but for now it seems like we still support 6.

I've tested this manually on Mac OS, but don't have a handy Windows machine or VM to test the UNC path stuff on right now.

How to test it

Scripting a test case was beyond me, but this is what I did. First create a ~/.sbt/repositories file containing something like the following (edit path, obviously):

[repositories]
  local
  foo: file:///Users/copumpkin/Sandbox/ivy-test/, [organization]/[module]/(scala_[scalaVersion]/)(sbt_[sbtVersion]/)[revision]/[type]s/[artifact](-[classifier]).[ext]

Then take an sbt project and change its publishTo to use your repository:

publishTo := (appResolvers in LocalRootProject).value.toSeq.flatten.find(_.name == "foo")

and attempt to publish. Without this change, it will fail saying things like:

[error] (core/*:publish) java.lang.UnsupportedOperationException: URL repository only support HTTP PUT at the moment

@eed3si9n eed3si9n added the ready label Sep 7, 2014
@typesafe-tools
Copy link

Can one of the admins verify this patch?

@copumpkin
Copy link
Contributor Author

The failure in travis build 829.2 looks spurious to me

@copumpkin copumpkin changed the title Support publishing to repositories specified in ~/.sbt/repositories. Fixes #1570 Support publishing to file repositories specified in ~/.sbt/repositories. Fixes #1570 Sep 7, 2014
@jsuereth
Copy link
Member

jsuereth commented Sep 7, 2014

Most travis failures are spurious. We have a hell of a time with it. It looks to be green to me now.

case "file" =>
// This hackery is to deal suitably with UNC paths on Windows. Once we can assume Java7, Paths should save us from this.
val file = try { new File(baseURL.toURI) } catch { case e: java.net.URISyntaxException => new File(baseURL.getPath) }
(file.getCanonicalPath, new FileSystemResolver)
Copy link
Member

Choose a reason for hiding this comment

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

Not 100% sure this part is needed, I'm also worried about ivy-interactions in its own cache about this.

e.g. I think this means the sbt-launch.jar will copy over files from local repos into the cache. Probably innocuous, but I've said that phrase too many times and been bitten.

I'm willing to try it out, but we should ping people who use file repos w/ the launcher, like Play/Activator , for testing.

@jsuereth
Copy link
Member

jsuereth commented Sep 7, 2014

Generally the fix looks good! There's always wierd issues between launcher version of Ivy + the builds, but I'd be ok merging this into the 0.13 branch and we can deal with what we find in 0.13.7 milestones later.

Thanks for the patch.

@copumpkin
Copy link
Contributor Author

I can remove it from boot/Update.scala if you'd like 😄

In general, this should only make a difference when publishing, and I don't think people publish much during boot (right!?)

I was hoping it would fix my other issue (which would make it more valuable during boot) but it doesn't seem to, so I'm going to look into what causes that separately.

If you would like me to change it, do you prefer a squished single commit with the change, or are you more of a proponent of accurate history?

@jsuereth
Copy link
Member

jsuereth commented Sep 7, 2014

@eed3si9n do we have a location for Notes for 0.13.7 yet? If not, we'll add em later.

What is your other issue? May be related to some other things. We generally prefer squished single commit with a commit message that leaves tidbits for maintainers, but I'll take what you got.

So yeah, let's remove the bit from boot/Update with reasoning in the commit message.

@copumpkin
Copy link
Contributor Author

Okay, I'll push the change this evening when I get home.

My other issue is http://stackoverflow.com/questions/25428309/sbt-ivy-failing-to-resolve-wildcard-ivy-dependencies-on-a-filesystem-resolver. I've since been able to get more verbose output from it and ivy seems to claim my dependencies are "unreachable" which generally confuses me. I don't think it's sbt-related, but I was hoping that switching the URL resolver to a File resolver would get it over its weird hump.

If you're curious, the code in question in ivy is http://grepcode.com/file/repo1.maven.org/maven2/org.apache.ivy/ivy/2.3.0/org/apache/ivy/plugins/resolver/BasicResolver.java#696.

I can submit a separate sbt ticket explaining how to reproduce it, but I'm not even sure it's an sbt bug at this point so I was holding off.

@copumpkin
Copy link
Contributor Author

Okay, we should be all set! 😤

@jsuereth
Copy link
Member

jsuereth commented Sep 8, 2014

@copumpkin Please open another issue. Even if it's a bug in Ivy, it's our responsibility to fix it.

Ivy tends to treat HTTP spec as a 'given' for a lot of services, but unfortunately URLs aren't HTTP, so a lot of that code is suspect.

This PR looks good as is.

jsuereth added a commit that referenced this pull request Sep 8, 2014
Support publishing to file repositories specified in ~/.sbt/repositories. Fixes #1570
@jsuereth jsuereth merged commit 9a60ca7 into sbt:0.13 Sep 8, 2014
@eed3si9n eed3si9n removed the ready label Sep 8, 2014
@copumpkin
Copy link
Contributor Author

@jsuereth okay, I'll cook up a repro this evening, thanks!

Is this commit going to make it into 0.13.6, or will it be 0.13.7? Couldn't quite tell from your wording earlier.

@copumpkin
Copy link
Contributor Author

@jsuereth I'll take the silence to mean that it's a 0.13.7 thing 😄

@jsuereth
Copy link
Member

0.13.6 is already in RC, which means only regressions would have made it in. 0.13.7 will have a shortened schedule though, as 0.13.6 had a really long RC cycle. I'll probably be issuing a 0.13.7-milestone for folks to test when 0.13.6 final comes out.

@copumpkin
Copy link
Contributor Author

Excellent, thanks!

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.

None yet

4 participants