vouch-opensource / krell

Simple ClojureScript React Native Tooling
Eclipse Public License 1.0
675 stars 37 forks source link

Windows path issues #91

Closed swannodette closed 3 years ago

swannodette commented 4 years ago

Currently we rely on the fact that the paths return from the REPL client can be used to access the file system. These should be converted into platform specific relative paths.

tarnung commented 4 years ago

There are two different cases of paths I encountered: Most of them lack the signified target directory. E.g. clojure/string.js instead of target/clojure/string.js or my_project/core.js instead of target/my_project/core.js The google closure files come in the form of targetgoog/string/stringformat.js instead of target/goog/string/stringformat.js

In repl.clj in fn send-file-loop I can replace value in (send-file repl-env (io/file value) load-file-req)) with (cond (.startsWith value "target/") value (.startsWith value "target") (string/replace value "target" "target/") :else (str "target/" value)) to hack my way around the problem.

I then get no more FileNotFoundExceptions. But the android emulator still shows "Waiting for Krell to load files".

Wheter I get the FileNotFoundExceptions or not, I do get a repl prompt and I can send an js/alert which shows up on the emulator.

tarnung commented 4 years ago

Maybe it's more a curiority than an actual practical development setup, but the following works:

Duplicate your repo. One on the windows file system, one on wsl2. Start krell in the wsl repo. In the windows repo run react native run-android -> Your emulator on windows is started, krell connects to it from wsl. Edit the files in your wsl repo and krell reloads them for you.

Starting krell on wsl in the mounted windows file system does not work. Probably a file watcher issue. Also It is slow and generally seems like a bad idea :) Starting react-native run-android on wsl isn't able to start an emulator on the windows side of things. Starting react-native run-android on windows in the wsl repo which is visible as a network drive isn't possible either. Hence the need for the duplicated repo.

swannodette commented 4 years ago

@tarnung I think in order to debug this we need some more logging information. Making some change to enable that then it would great to get your shell & device output.

tarnung commented 4 years ago

Logged info in krell terminal (for rn-terminal output see next post):

Load file: target\goog\base.js Load file: target\goog\deps.js Load file: target\krell_repl_deps.js Load file: target\cljs_deps.js Load file: target\goog\dom\nodetype.js Load file: target\goog\dom\htmlelement.js Load file: target\goog\fs\url.js Load file: target\goog\i18n\bidi.js Load file: target\goog\reflect\reflect.js Load file: target\goog\object\object.js Load file: target\goog\html\trustedtypes.js Load file: target\goog\string\internal.js Load file: target\goog\string\typedstring.js Load file: target\goog\debug\error.js Load file: target\goog\string\stringbuffer.js Load file: target\goog\functions\functions.js Load file: target\goog\labs\useragent\util.js Load file: target\goog\dom\tags.js Load file: target\goog\asserts\asserts.js Load file: target\goog\dom\tagname.js Load file: target\goog\math\integer.js Load file: target\goog\array\array.js Load file: target\goog\dom\asserts.js Load file: target\goog\math\long.js Load file: target\goog\string\const.js Load file: target\goog\math\math.js Load file: target\goog\structs\structs.js Load file: target\goog\html\safescript.js Load file: target\goog\html\trustedresourceurl.js Load file: target\goog\labs\useragent\browser.js Load file: target\goog\iter\iter.js Load file: target\goog\html\safeurl.js Load file: target\goog\html\safestyle.js Load file: target\goog\structs\map.js Load file: target\goog\html\safestylesheet.js Load file: target\goog\html\safehtml.js Load file: target\goog\html\uncheckedconversions.js Load file: target\goog\dom\safe.js Load file: target\goog\string\string.js Load file: target\goog\uri\utils.js Load file: target\goog\uri\uri.js Load file: target\cljs\core.js

Then it explodes, searching files without the "target\" prefix. (The files exist in the target folder)

ClojureScript 0.0.1108670661 cljs.user=> #error { :cause File clojure\string.js does not exist :data {} :via [{:type clojure.lang.ExceptionInfo :message File clojure\string.js does not exist :data {} :at [krell.repl$send_file invokeStatic repl.clj 72]}] :trace [[krell.repl$send_file invokeStatic repl.clj 72] [krell.repl$send_file invoke repl.clj 61] [krell.repl$send_file invokeStatic repl.clj 63] [krell.repl$send_file invoke repl.clj 61] [krell.repl$send_file_loop$fn10408 invoke repl.clj 79] [krell.repl$send_file_loop invokeStatic repl.clj 77] [krell.repl$send_file_loop invoke repl.clj 74] [krell.repl$setup$fn10475 invoke repl.clj 286] [clojure.lang.AFn applyToHelper AFn.java 152] [clojure.lang.AFn applyTo AFn.java 144] [clojure.core$apply invokeStatic core.clj 665] [clojure.core$with_bindingsSTAR invokeStatic core.clj 1973] [clojure.core$with_bindingsSTAR doInvoke core.clj 1973] [clojure.lang.RestFn invoke RestFn.java 425] [clojure.lang.AFn applyToHelper AFn.java 156] [clojure.lang.RestFn applyTo RestFn.java 132] [clojure.core$apply invokeStatic core.clj 669] [clojure.core$bound_fnSTAR$fn__5749 doInvoke core.clj 2003] [clojure.lang.RestFn invoke RestFn.java 397] [clojure.lang.AFn run AFn.java 22] [java.lang.Thread run Thread.java 748]]}

error {

:cause File reagent\impl\util.js does not exist :data {} :via [{:type clojure.lang.ExceptionInfo :message File reagent\impl\util.js does not exist :data {} :at [krell.repl$send_file invokeStatic repl.clj 72]}] :trace [[krell.repl$send_file invokeStatic repl.clj 72] [krell.repl$send_file invoke repl.clj 61] [krell.repl$send_file invokeStatic repl.clj 63] [krell.repl$send_file invoke repl.clj 61] [krell.repl$send_file_loop$fn10408 invoke repl.clj 79] [krell.repl$send_file_loop invokeStatic repl.clj 77] [krell.repl$send_file_loop invoke repl.clj 74] [krell.repl$setup$fn10475 invoke repl.clj 286] [clojure.lang.AFn applyToHelper AFn.java 152] [clojure.lang.AFn applyTo AFn.java 144] [clojure.core$apply invokeStatic core.clj 665] [clojure.core$with_bindingsSTAR invokeStatic core.clj 1973] [clojure.core$with_bindingsSTAR doInvoke core.clj 1973] [clojure.lang.RestFn invoke RestFn.java 425] [clojure.lang.AFn applyToHelper AFn.java 156] [clojure.lang.RestFn applyTo RestFn.java 132] [clojure.core$apply invokeStatic core.clj 669] [clojure.core$bound_fnSTAR$fn__5749 doInvoke core.clj 2003] [clojure.lang.RestFn invoke RestFn.java 397] [clojure.lang.AFn run AFn.java 22] [java.lang.Thread run Thread.java 748]]}

There are a lot more similar Exceptions that I cut. I can paste the full list if it helps.

tarnung commented 4 years ago

rn-terminal output:

[Tue May 19 2020 16:50:25.546] BUNDLE ./index.js

[Tue May 19 2020 16:50:28.664] LOG Running "ll" with {"rootTag":1} [Tue May 19 2020 16:50:28.665] LOG Connected to Krell REPL Server [Tue May 19 2020 16:50:29.780] LOG Krell cache is up-to-date [Tue May 19 2020 16:50:29.387] LOG CACHE PUT: target\goog\base.js 1573126578000 [Tue May 19 2020 16:50:30.229] LOG CACHE PUT: target\goog\deps.js 1589899829145 [Tue May 19 2020 16:50:30.253] LOG CACHE PUT: target\krell_repl_deps.js 1589899829148 [Tue May 19 2020 16:50:30.286] LOG CACHE PUT: target\cljs_deps.js 1589899810716 [Tue May 19 2020 16:50:30.319] LOG CACHE PUT: target\goog\dom\nodetype.js 1573126578000 [Tue May 19 2020 16:50:30.328] LOG CACHE PUT: target\goog\dom\htmlelement.js 1573126578000 [Tue May 19 2020 16:50:30.336] LOG CACHE PUT: target\goog\fs\url.js 1573126578000 [Tue May 19 2020 16:50:30.361] LOG CACHE PUT: target\goog\i18n\bidi.js 1573126578000 [Tue May 19 2020 16:50:30.369] LOG CACHE PUT: target\goog\reflect\reflect.js 1573126578000 [Tue May 19 2020 16:50:30.385] LOG CACHE PUT: target\goog\object\object.js 1573126578000 [Tue May 19 2020 16:50:30.393] LOG CACHE PUT: target\goog\html\trustedtypes.js 1573126578000 [Tue May 19 2020 16:50:30.404] LOG CACHE PUT: target\goog\string\internal.js 1573126578000 [Tue May 19 2020 16:50:30.413] LOG CACHE PUT: target\goog\string\typedstring.js 1573126578000 [Tue May 19 2020 16:50:30.420] LOG CACHE PUT: target\goog\debug\error.js 1573126578000 [Tue May 19 2020 16:50:30.429] LOG CACHE PUT: target\goog\string\stringbuffer.js 1573126578000 [Tue May 19 2020 16:50:30.440] LOG CACHE PUT: target\goog\functions\functions.js 1573126578000 [Tue May 19 2020 16:50:30.452] LOG CACHE PUT: target\goog\labs\useragent\util.js 1573126578000 [Tue May 19 2020 16:50:30.486] LOG CACHE PUT: target\goog\dom\tags.js 1573126578000 [Tue May 19 2020 16:50:30.512] LOG CACHE PUT: target\goog\asserts\asserts.js 1573126578000 [Tue May 19 2020 16:50:30.540] LOG CACHE PUT: target\goog\dom\tagname.js 1573126578000 [Tue May 19 2020 16:50:30.587] LOG CACHE PUT: target\goog\math\integer.js 1573126578000 [Tue May 19 2020 16:50:30.680] LOG CACHE PUT: target\goog\array\array.js 1573126578000 [Tue May 19 2020 16:50:30.717] LOG CACHE PUT: target\goog\dom\asserts.js 1573126578000 [Tue May 19 2020 16:50:30.739] LOG CACHE PUT: target\goog\math\long.js 1573126578000 [Tue May 19 2020 16:50:30.753] LOG CACHE PUT: target\goog\string\const.js 1573126578000 [Tue May 19 2020 16:50:30.775] LOG CACHE PUT: target\goog\math\math.js 1573126578000 [Tue May 19 2020 16:50:30.806] LOG CACHE PUT: target\goog\structs\structs.js 1573126578000 [Tue May 19 2020 16:50:30.827] LOG CACHE PUT: target\goog\html\safescript.js 1573126578000 [Tue May 19 2020 16:50:30.864] LOG CACHE PUT: target\goog\html\trustedresourceurl.js 1573126578000 [Tue May 19 2020 16:50:30.885] LOG CACHE PUT: target\goog\labs\useragent\browser.js 1573126578000 [Tue May 19 2020 16:50:30.959] LOG CACHE PUT: target\goog\iter\iter.js 1573126578000 [Tue May 19 2020 16:50:31.900] LOG CACHE PUT: target\goog\html\safeurl.js 1573126578000 [Tue May 19 2020 16:50:31.350] LOG CACHE PUT: target\goog\html\safestyle.js 1573126578000 [Tue May 19 2020 16:50:31.580] LOG CACHE PUT: target\goog\structs\map.js 1573126578000 [Tue May 19 2020 16:50:31.900] LOG CACHE PUT: target\goog\html\safestylesheet.js 1573126578000 [Tue May 19 2020 16:50:31.142] LOG CACHE PUT: target\goog\html\safehtml.js 1573126578000 [Tue May 19 2020 16:50:31.180] LOG CACHE PUT: target\goog\html\uncheckedconversions.js 1573126578000 [Tue May 19 2020 16:50:31.219] LOG CACHE PUT: target\goog\dom\safe.js 1573126578000 [Tue May 19 2020 16:50:31.305] LOG CACHE PUT: target\goog\string\string.js 1573126578000 [Tue May 19 2020 16:50:31.373] LOG CACHE PUT: target\goog\uri\utils.js 1573126578000 [Tue May 19 2020 16:50:31.456] LOG CACHE PUT: target\goog\uri\uri.js 1573126578000 [Tue May 19 2020 16:50:33.346] LOG CACHE PUT: target\cljs\core.js 1589553310287 [Tue May 19 2020 16:50:33.385] LOG Namespace ll.core not loaded, fetching: ll/core.js

tarnung commented 4 years ago

The emulator is still stuck at the "waiting for krell to load files" stage. Also if I (js/alert) I get ReferenceError: Can't find variable: cljs

swannodette commented 4 years ago

Apologies for the delay please try master

tarnung commented 4 years ago

No worries, we're not in a hurry here. Thanks for putting up with this issue at all!

Sadly I get the exact same behaviour as before. If i put my hacky cond mentioned above (prepending "target/") into krell.utils/platform-path, i can fix the FileNotFoundExceptions. But even then I'm still stuck at the "Waiting for Krell to load files" screen. If I change core.cljs in my project, the android-emulator refreshes but then shows an error screen "can't find variable goog"

swannodette commented 4 years ago

@tarnung if you're getting the same thing as before, it's likely something is wrong with your environment since we're not sending platform paths to the client anymore. Did you try with a fresh project?

tarnung commented 4 years ago

@swannodette Yes, fresh project and uninstalled the old app from emulator, so there should not be anything strange project-side. I don't think I have any strange system-wide settings that could influence this either. Anyway, maybe we have to wait for someone else that tries to use krell on windows so we can rule out environment-issues on my machine. Maybe I'll poke at the source code some more to see if I can locate the cause. If I find anything I'll let you know. Thanks again for looking into this!

swannodette commented 4 years ago

@tarnung and the logs are exactly the same as above? The import part is the CACHE PUT logs. Those should be forward slash now. Please paste exactly what you're seeing now with master - thanks!

tarnung commented 4 years ago

@swannodette Sorry for the silence. I finished my bachelor's degree over the last month. Using master the logs are exactly the same except for having / instead of \ as path separators. But that does not change the behaviour.

I messed around a bit more in the krell JS files. (Until now i had only modified the krell clojure files) I can get it to work if i make sure that there always is a "target/" prepended wherever a path is used like this:

const prependTargetToPath = (path)=>{
    if(path==undefined || path.startsWith("target/")){
        // do nothing
    } else if(path.startsWith("target")){
        path = path.replace("target", "target/");
    } else {
        path = "target/" + path;
    }
    return path;
}

I did observe the same types of bad paths on the js side as i've seen previously on the clojure side. Some paths don't have "target/" at all and some goog paths start with "targetgoog" instead of "target/goog".

I hope this helps to narrow down the problem. It's still possible that it's something about my setup, but i wouldn't know what.

I added one null-check and several uses of the path-formatting function. I did not verify whether i need the path-formatting function in all places listed below. I do need the null-check though.

I made the following changes:

UTIL.CLJ

(defn platform-path [rel-url-frag]
  (let [value (string/replace rel-url-frag "/" File/separator)]
    (cond
      (.startsWith value "target/") value
      (.startsWith value "target") (string/replace value "target" "target/")
      :else (str "target/" value))))

KRELL_REPL.JS

const isLoaded_ = (path, index) => {
    let ids = index[path];
    // I added this null-check
    if(!ids){
        return false;
    }
    for(let i = 0; i < ids.length; i++) {
        if(exists_(global, ids[i])) {
            return true;
        }
    }
    return false;
};

global.CLOSURE_IMPORT_SCRIPT = function(path, optContents) {
    path = prependTargetToPath(path)
    .....

const onSourceLoad = (path, cb) => {
    path = prependTargetToPath(path)
    .....

const handleMessage = (socket, data) => {
  .....
  if(req) {
    if(req.type === "load-file") {
      let path = prependTargetToPath(req.value);
  .....
  if (req) {
    notifyListeners({...req, value: prependTargetToPath(req.value)});
    .....

MAIN_DEV.JS

function krellUpdateRoot(cb) {
    waitForCore(function() {
        let xs = main.split(".");
        if(!exists(global, xs)) {
            let path = goog.debugLoader_.getPathFromDeps_(main);
            path = prependTargetToPath(path);
            .....
swannodette commented 4 years ago

This seems like fixing the symptom not the cause. I'd instead like to understand how it's possible that target gets lost given the client is caching paths with target.

tarnung commented 4 years ago

It very much was fixing the symptoms. I traced the symptoms a bit further up the chain. If i fix the paths originating from those two touching points with google closure, then everything works fine:

in main.dev.js -> krellUpdateRoot let path = goog.debugLoader_.getPathFromDeps_(main);

and in krel_repl.js the path argument passed to global.CLOSURE_IMPORT_SCRIPT = function(path, optContents) {...}

Since I don't know much about google closure I'm stuck there at the moment.

noonian commented 3 years ago

I dug into this issue a bit and I think I've discovered the root of the problem.

CLOSURE_BASE_PATH is being written out with backslashes on Windows (because .getPath returns platform specific path separators) https://github.com/vouch-opensource/krell/blob/master/src/krell/gen.clj#L49

So if output-dir is "target" CLOSURE_BASE_PATH will be "target\goog/", which is interpreted by JavaScript as "targetgoog/".

This messes up the cache logic because toPath is implemented with CLOSURE_BASE_PATH https://github.com/vouch-opensource/krell/blob/master/resources/krell_repl.js#L84

Here is a gist of react-native terminal output illustrating the issue. I added code to log out CLOSURE_BASE_PATH in the beginning and I also added logging to KRELL_CACHE.get to illustrate that the correct paths are being cached, but incorrect paths are used when attempting to lookup in the cache. https://gist.github.com/noonian/27754773897f4fade4b591fe25ba9a4d

The fix is to convert backslashes to forward slashes before writing out CLOSURE_BASE_PATH in krell.gen.

(defn rewrite-separator [s]
  (string/replace s "\\" "/"))

(defn write-repl-js
  "Write out the REPL support code. See resources/krell_repl.js"
  [repl-env opts]
  (let [source   (slurp (io/resource "krell_repl.js"))
        out-file (io/file (:output-dir opts) "krell_repl.js")]
    (util/mkdirs out-file)
    (write-if-different out-file
      (-> source
        (string/replace "$KRELL_VERBOSE" (str (or (-> repl-env :options :krell/verbose) false)))
        (string/replace "$KRELL_SERVER_IP" (net/get-ip))
        (string/replace "$KRELL_SERVER_PORT" (-> repl-env :options :port str))
        (string/replace "$CLOSURE_BASE_PATH"
                        #_(str (.getPath (io/file (:output-dir opts) "goog")) "/") ;current impl
                        (rewrite-separator                                         ;this is the fix
                         (str (.getPath (io/file (:output-dir opts) "goog")) "/")))))))

With this hack in place, everything worked flawlessly and I was able to get the reagent tutorial working with code reloading on Windows.

swannodette commented 3 years ago

@noonian thank you! Give master a try.

tarnung commented 3 years ago

Thanks @noonian for the research. Master seems to work for me. I'll try it out more extensively later.

noonian commented 3 years ago

Master works for me! @tarnung thank you for doing the initial investigation. Your comments and the discussion here was very helpful as I started digging in.

noonian commented 3 years ago

The same issue occurs when writing out krell_assets.js, which breaks js/require for assets on Windows. Replacing File/separator with "/" in krell.passes/rewrite-asset-requires fixes it (or in krell.util/get-path).

https://github.com/vouch-opensource/krell/blob/master/src/krell/passes.clj#L39

swannodette commented 3 years ago

@noonian try master

noonian commented 3 years ago

@swannodette thanks, requiring assets works now on master

swannodette commented 3 years ago

I think we can close this one now, if there's more specific issues we can open separate tickets.