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

Redundant Object Key Slashes Preventing Resolution #52

Closed
michaelahlers opened this issue Sep 22, 2017 · 9 comments · Fixed by #53
Closed

Redundant Object Key Slashes Preventing Resolution #52

michaelahlers opened this issue Sep 22, 2017 · 9 comments · Fixed by #53
Assignees
Labels

Comments

@michaelahlers
Copy link

michaelahlers commented Sep 22, 2017

With 0.17.0 (I think; haven't seen this with 0.16.0), S3 object keys contain an extra slash that prevents resolution.

Log messages like the following appear:

[warn] ==== My Repository: tried
…
[warn]   -- artifact my-organization#my-artifact_2.11;1.0.0!my-artifact_2.11.jar:
[warn]   s3://my-bucket/ivy2/my-organization/my-artifact_2.11//1.0.0/jars/my-artifact_2.11.jar
…

For S3, these are not normalized. The CLI, for example, produces different results:

$ aws s3 ls 's3://my-bucket/ivy2/my-organization/my-artifact_2.11//1.0.0/'
$ aws s3 ls 's3://my-bucket/ivy2/my-organization/my-artifact_2.11/1.0.0/'
                           PRE docs/
                           PRE ivys/
                           PRE jars/
                           PRE srcs/

SBT build.properties:

sbt.version=1.0.2

Plugin added with:

resolvers += Resolver.jcenterRepo
addSbtPlugin("ohnosequences" % "sbt-s3-resolver" % "0.17.0")

Resolver defined as:

resolvers += s3resolver.value("My Repository", s3("my-bucket/ivy2")).withIvyPatterns

(Logs truncated and specifics redacted as appropriate.)

@michaelahlers
Copy link
Author

Should I move this to ohnosequences/ivy-s3-resolver?

@laughedelic laughedelic self-assigned this Sep 22, 2017
@michaelahlers
Copy link
Author

michaelahlers commented Sep 22, 2017

ohnosequences.ivy.S3Repository#getResource called from org.apache.ivy.plugins.resolver.RepositoryResolver#findResourceUsingPattern in Ivy 2.4.0 is given the URI with redundant delimiters. If I break on that function, edit the value in flight, I can get these resolving. One possible fix is to use java.net.URI to normalize the path, but I'm not sure what unintended consequences that'll have.

@laughedelic
Copy link
Contributor

@michaelahlers Thanks for reporting and debugging. Are you sure v0.16.0 worked with your repository? Because I didn't touch any code related to this.

One possible fix is to use java.net.URI to normalize the path, but I'm not sure what unintended consequences that'll have.

It's a good idea. I'm pretty sure S3 paths can't contain double slash.

I still don't understand though where it comes from. I will take a closer look tonight.

@michaelahlers
Copy link
Author

michaelahlers commented Sep 22, 2017

@laughedelic, it may be a problem with SBT. Forgot to mention this with 1.0.2, which may have changed the resolver's behavior (updated my earlier comment to mention the Ivy version in use).

@michaelahlers
Copy link
Author

Publishing ivy-s3-resolver (and sbt-s3-resolver depending on it) locally with ohnosequences/ivy-s3-resolver#19 definitely fixes the bug.

@laughedelic
Copy link
Contributor

laughedelic commented Sep 22, 2017

it may be a problem with SBT. Forgot to mention this with 1.0.2, which may have changed the resolver's behavior

This is correct! 🎯 I just published (locally) v0.17.0 for sbt-0.13, tried to publish and it went fine. But with sbt-1.0+ it publishes (and resolves) with that extra slash.

On another note, I discovered that S3 treats this extra slash as a separate object. So if you have

publishTo := Some(s3resolver.value("Test repo", s3("test-repo")).withIvyPatterns)

The it will publish it to s3://test-repo/org/name_2.11//0.1-SNAPSHOT/...:

$ aws s3 ls s3://test-repo/org/artifact_2.11/
                           PRE /
$ aws s3 ls s3://test-repo/org/artifact_2.11//
                           PRE 0.1-SNAPSHOT/

Which is complete nonsense if you ask me. But this is how S3 works.


Anyway, thanks for noticing this and fixing 👍 I'm going to merge your workaround and cut a hotfix release.

@michaelahlers
Copy link
Author

It's a bit funny; a coworker and I noticed that publishing with that redundant delimiter does work and—because S3 is a key-value store with no concept of paths—it accepts it and the UI conveniently presents it as an empty segment. Doh! Cool we nailed this one fast. Looking forward to that new version!

@laughedelic
Copy link
Contributor

laughedelic commented Sep 22, 2017

Publishing ivy-s3-resolver (and sbt-s3-resolver depending on it) locally with ohnosequences/ivy-s3-resolver#19 definitely fixes the bug.

Are you sure about this? I tried it out and I still observe the problem. I think that it doesn't change the behaviour of publishing anyway, plus for resolving, I think malformed URI is still passed to the list command.

Before patching all things in ivy-s3-resolver, I'd like to figure out where does this malformed URI come from originally (and why it changed in sbt-1.0) and probably then we can fix this better.

@laughedelic
Copy link
Contributor

laughedelic commented Sep 22, 2017

OK. I found where is the problem. As far I understand in sbt-1.0 they started using sbt/librarymanagement. And there they changed Resolver.localBasePattern (which is used for ivyStylePatterns): 0.13 vs. 1.0

[organisation]/[module]/(scala_[scalaVersion]/)(sbt_[sbtVersion]/)[revision]/[type]s/[artifact](-[classifier]).[ext])
[organisation]/[module]/(scala_[scalaVersion]/)(sbt_[sbtVersion]/)(/[branch])/[revision]/[type]s/[artifact](-[classifier]).[ext])
                                                                             ^

Besides that there are some new segments, there is this extra / before [revision]. So that when the scala_[scalaVersion]/ segment is present and the following two not (our case), we get scala_[scalaVersion]//[revision].

I'm going to fix it here let's improve your fix in ohnosequences/ivy-s3-resolver#19 and I will submit a PR to sbt/librarymanagement.

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

Successfully merging a pull request may close this issue.

2 participants