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

Relative paths throws exception on Window #15

Closed
jjttjj opened this issue Feb 23, 2017 · 6 comments
Closed

Relative paths throws exception on Window #15

jjttjj opened this issue Feb 23, 2017 · 6 comments

Comments

@jjttjj
Copy link
Contributor

jjttjj commented Feb 23, 2017

The relative-path code in s3_deploy.clj here:

  (string/replace (.getCanonicalPath f)
                  (re-pattern (str (.getCanonicalPath dir) "/"))
                  ""))

with

                                        C:\Users\x\y/
                                           ^
    description: "Illegal/unsupported escape sequence"
          index: 3

fails on windows. Because the canonical path of windows files include only single-escaped backslashes, like c:\\path\\to\\file, while re-pattern requires extra escapes, like c:\\\\path\\\\to\\\\file

I'm not sure if this is the cleanest fix, but one option is to change the relative-path code to the following:

(defn relative-path [dir f]
  (-> dir .toURI (.relativize (.toURI f)) .getPath))

This should work on all OSes. Let me know if you have any thoughts or would like a PR.

@martinklepsch
Copy link
Member

@jjttjj Hey Justin, that looks like a much better implementation either way. Would you like to open a PR? Otherwise I can also just apply the change, just let me know :)

jjttjj added a commit to jjttjj/s3-deploy that referenced this issue Feb 23, 2017
@jjttjj
Copy link
Contributor Author

jjttjj commented Feb 23, 2017

I just opened a PR!

@martinklepsch
Copy link
Member

The PR looks great, just merged and will cut a release in a sec! Thanks a lot.

Out of curiosity would you mind telling me more about how you use this lib, whether you also use Confetti itself etc? Thanks :)

@martinklepsch
Copy link
Member

Just deployed [confetti/s3-deploy "0.1.2"] — will also cut a confetti release in a sec.

@jjttjj
Copy link
Contributor Author

jjttjj commented Feb 24, 2017

Thanks for this, and for your work on Confetti!

I've basically been just trying to use Confetti to deploy a simple static site, and I switch between Windows and Linux. There are still problems somewhere along my stack on Windows, but this seemed like some low hanging fruit. I'll report any further windows issues as I come to understand them and verify that they are in fact confetti things and not boot, etc.

Incidentally I have some vague far off goals of deploying sites to S3 though my own UI, but I don't have a great understanding of AWS in general or using it from clojure yet. So I'm not really sure yet if s3-deploy will be ideal for that or if I'd be better off working with amazonica directly.

But for now Confetti has been great for what I'm doing.

@martinklepsch
Copy link
Member

I hope you got to the deploy part eventually? Sorry about the Windows troubles, don't use Windows myself and thus don't quite have the awareness what could go wrong there..

Incidentally I have some vague far off goals of deploying sites to S3 though my own UI, but I don't have a great understanding of AWS in general or using it from clojure yet. So I'm not really sure yet if s3-deploy will be ideal for that or if I'd be better off working with amazonica directly.

I think s3-deploy is suitable for any situation where you want to synchronize or freshly upload files to S3 in an efficient way while also providing user feedback. What you're planning to do sounds like the kind of thing I had in mind when I wrote the library so if you get yo that part of your project feel free to ask for ideas/code-walkthroughs etc. Documentation could be improved admittedly :)

I'm also on the Clojurians slack (http://clojurians.net), feel free to ping me there.

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

2 participants