Origen-SDK / o2

MIT License
4 stars 0 forks source link

CLI issue #101

Closed pderouen closed 4 years ago

pderouen commented 4 years ago

Now that I can reliably launch origen inside the example app, I tried the generate command. I'm not sure if it's user error or another bug similar to #27. I tried 3 different ways.

> origen g toggle

 line 105, in __origen__
    _origen.file_handler().init(files)
OSError: The system cannot find the file specified. (os error 2) - toggle

> origen g toggle.py

OSError: The system cannot find the file specified. (os error 2) - toggle.py

> origen g .\example\patterns\toggle.py

OSError: The filename, directory name, or volume label syntax is incorrect. (os error 123) - .\example\patterns oggle.py
pderouen commented 4 years ago

The issue (I think) is centered around the Windows folder separation character being the same as the escape character.

The file list is read by the CLI correctly and passed to the python launcher correctly, and then passed back to the core Rust code incorrectly (note "\" changed to "\\", except for "\t" in "\toggle"; "\t" is getting turned into a tab character):

> origen g .\example\patterns\toggle.py

 # this is the command that gets sent to poetry to launch python
cmd is: from origen.boot import __origen__; __origen__('generate', files=['.\example\patterns\toggle.py']);

# this is from boot.py, right before sending the file name to origen core Rust code
#   should be '.\\example\\patterns\\toggle.py' to work correctly; no idea why the
#   last "\" isn't turned into "\\"

handling file init
['.\\example\\patterns\toggle.py']
In file_handler::FileHandler::init files are: [
    ".\\example\\patterns\toggle.py",
]

# on the Rust side \t is turned into tab resulting in an error when canonicalize is called
# notice patterns\toggle.py is now patterns oggle.py

OSError: The filename, directory name, or volume label syntax is incorrect. (os error 123) - .\example\patterns oggle.py

I can get further along in the generate process by manually replacing "\" with "/".

pderouen commented 4 years ago

A simple fix would be to change the folder separator characters to the unix style "/" from the CLI before calling Python. Would a global replace of all "\" into "/" for files introduce problems?

coreyeng commented 4 years ago

There is no pattern dispatcher, so you need to give it the path.

The screwiness though is due to the roundabout trip from Python, to Rust, then back to Python again. The "\" get escaped on the first Python to Rust boundary, Rust then treats it all as raw string, then when it tries to send it back to Python, Python doesn't know what to do with a string with bunch of escape characters that aren't escaping anything. Give it a Linux-style path and it'll work.

Keep in mind, this is still in development. I know the problem, but its only an issue for Windows and I was the only one working on Windows up til now. Its works as expected in Linux. I've got this as a to-do item for me when I start on a pattern dispatcher, but I wanted to see if we'd get plugins going before I did a pattern dispatcher. And I've been spending much of my "O2 time" on the docs recently.

I think we really just need to overhaul how paths are handled. I started that with the pypath macro to translate a PathBuf into a pathlib.Path, but I haven't fanned that out to everything yet.

I don't think a straight find and replace is the best way to go. Instead, I think we just want to always handle paths as a pathlib.Path on the Python side, and as a PathBuf on the Rust side. Then, it really just becomes ensuring that they translate properly across the boundary, which I think I already have for Rust -> Python. Missing the other direction though.

I started #102 for general pattern generation stuff, so if you'd like to fix up the paths, just make a note of it there. I've obviously not been too concerned with the Windows path issues so far... lol.

pderouen commented 4 years ago

Ok. I'm perfectly fine using linux style paths.

pderouen commented 4 years ago

The bug can also be fixed by prepending the file string with r to force the python interpreter to treat the file strings as literals instead of trying to process escape characters. See #100

pderouen commented 4 years ago

fixed in #100