Origen-SDK / o2

MIT License
4 stars 0 forks source link

New app #120

Closed ginty closed 3 years ago

ginty commented 3 years ago

Adds an 'origen new' command for building a new application and for generating content (e.g. a new dut) within an application.

The generated content could be better and will evovle over time, however this should establish the infrastructure for both features allowing further expantsion by the community.

All templates reside in rust/origen/cli/src/commands/new/templates.

In the case of the new app it is simply a case of modifying those file and/or addition additional files into the template directory and they will be picked up.

coreyeng commented 3 years ago

Hi @ginty,

I fixed the subprojects (on Windows at least) during web build. I hope you don't mind, but I threw it onto here. It seems to be tied to the VIRTUAL_ENV environment variable. Previously, the virtualenvs all pointed to the same thing but that seems to not be the case. When using subprocess.run, the VIRTUAL_ENV is copied over, which, when run the python/ app, means that the path ../../python doesn't exist and crashes the build.

Back to the new app though, I also ran into a small issue with running from a local origen build. It'll find the origen module, but it doesn't find the _origen library in that directory. I worked around it by setting the PYTHONPATH in the app though. Not sure if you've tried this. Could just be a Windows thing too. I'll keep looking at it. @pderouen, this may affect you as well.

Thanks!

coreyeng commented 3 years ago

I think I got the web stuff working on Linux now. Python 3.6 throws a warning as it doesn't like the fanciness in making an abstract class method a property, but I envision we'll eventually tie the website release to Travis and then we can spin the docs with whatever setup we want. It seems fine in 3.8, though it could also be re-written without much effort since the compiler is still small.

This should take of #119 too.

ginty commented 3 years ago

Hi @coreyeng,

Back to the new app though, I also ran into a small issue with running from a local origen build. It'll find the origen module, but it doesn't find the _origen library in that directory. I worked around it by setting the PYTHONPATH in the app though. Not sure if you've tried this. Could just be a Windows thing too. I'll keep looking at it. @pderouen, this may affect you as well.

I noticed this too and was plannig to fix it on another branch where I'm setting up the Python plugin system. FYI the changes I was planning to make there are as follows:

When the app is configured for Origen development the origen build command will become available within the app and it will work the same way as in an o2 workspace - i.e. just run it from within the app workspace and it will compile _origen and link it into the app.

That should avoid any need to mess around with PYTHONPATH.

Let me know if you have any comments/requests on this.

Thanks!

ginty commented 3 years ago

Hi @coreyeng,

Thanks for the update here.

Regarding this item:

I think I got the web stuff working on Linux now. Python 3.6 throws a warning as it doesn't like the fanciness in making an abstract class method a property, but I envision we'll eventually tie the website release to Travis and then we can spin the docs with whatever setup we want.

Can we make it 3.6 compatible?

Understood that as far as the O2 docs are concerned then limiting to 3.8 might not be that bad, but for app docs and for users wanted to build the o2 docs locally (i.e. like me!), I think we should be supporting 3.6. I still think we should be supporting 3.6 as a minimum version since that is default on Ubuntu/WSL2.

Thanks!

coreyeng commented 3 years ago

Hi @ginty,

What you have planned for environment sounds good! I wasn't sure if it was user error, windows problems, or something else. Setting the PYTHONPATH as a workaround for now is fine for me though.

I added a workaround for Python 3.6. I have too much hubris and was quite giddy when I got the abstractclassmethod to work as a property like I wanted. It works functionally (all the specs pass) and I'll be damned if 3.6's docstrings are what messes it up! :p Instead, I added an event which may come in handy later which allows preprocessing of the docstrings. I just override it there.

Thanks,

Corey