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

README+Docs: Add virtual environments as recommended usage way #176

Conversation

EverythingElseWasAlreadyTaken
Copy link
Collaborator

@EverythingElseWasAlreadyTaken EverythingElseWasAlreadyTaken commented May 7, 2024

Add description how to use FABulous with virtual environments in README and docs, since it is recommended for working with python projects.

Introduce $, (venv)$ and FABulous> as commandprompt prefixes.

@KelvinChung2000
Copy link
Collaborator

Since you are doing the document, could you add a line about FASM installation? The standard installation will yield a warning.

@KelvinChung2000
Copy link
Collaborator

When I try to install with pip install -e . I also noticed that the tool fails with the following error.

has a 'pyproject.toml' and its build backend is missing the 'build_editable' hook. Since it does not have a 'setup.py' nor a 'setup.cfg', it cannot be installed in editable mode. Consider using a build backend that supports PEP 660.

@IAmMarcelJung
Copy link
Collaborator

Are you executing it inside a venv? I also got this error, but only when using it with the global python installation.

@KelvinChung2000
Copy link
Collaborator

I am doing a global installation. I will do it the venv way. The document will also need to mention this then.

@EverythingElseWasAlreadyTaken
Copy link
Collaborator Author

I am doing a global installation. I will do it the venv way. The document will also need to mention this then.

For this reason I made this PR :)

It is generally no longer recommended to install python packages with pip globally, since it can break Python system packages and can cause problems with shared dependencies.

In newer Python versions (>=3.11) it will throw an error, if you try to install any packages globally with pip.
You can still bypass this error, but it is generally recommended to use venvs.

@EverythingElseWasAlreadyTaken
Copy link
Collaborator Author

EverythingElseWasAlreadyTaken commented May 8, 2024

When I try to install with pip install -e . I also noticed that the tool fails with the following error.

has a 'pyproject.toml' and its build backend is missing the 'build_editable' hook. Since it does not have a 'setup.py' nor a 'setup.cfg', it cannot be installed in editable mode. Consider using a build backend that supports PEP 660.

This issue seems to be caused by an old version of setuptools.
That would explain why it works on my machine when I install FABulous globally.

I've updated the pyproject.toml, could you please try to install it again globally from this branch?

@KelvinChung2000
Copy link
Collaborator

If the global install is deprecated, should we only stick to the venv installation?

@IAmMarcelJung
Copy link
Collaborator

I still get the error on global install, even with the setuptools>=64 requirement.

We could also change the documentation from recommending a venv to making a venv a prerequiste, since it is clearly a better way to install the package, for the reasons you mentioned @EverythingElseWasAlreadyTaken .

@EverythingElseWasAlreadyTaken
Copy link
Collaborator Author

I still get the error on global install, even with the setuptools>=64 requirement.

Maybe you have to update setuptools manually and also update pip to make this work.
At least this is what people with similar issues did here: pypa/setuptools#4037

We could also change the documentation from recommending a venv to making a venv a prerequiste, since it is clearly a better way to install the package, for the reasons you mentioned @EverythingElseWasAlreadyTaken .

The venv instaallation is the only way described in the documentation.
The global installation isn't described anymore.

@EverythingElseWasAlreadyTaken
Copy link
Collaborator Author

Since you are doing the document, could you add a line about FASM installation? The standard installation will yield a warning.

Should i just add a Note, that this warning is normal? Or is there a way to fix it?

@KelvinChung2000
Copy link
Collaborator

To make the warning disappear, we will need to install fasm from the source. I think just putting a note is all we need.

@IAmMarcelJung
Copy link
Collaborator

The venv instaallation is the only way described in the documentation. The global installation isn't described anymore.

Okay then I will not try to make the global installation work since I won't use it personally.

Add description how to use FABulous with virtual environments in README and docs,
since it is recommended for working with python projects.

Introduce $, (venv)$ and FABulous> as commandprompt prefixes.

Signed-off-by: Jonas K. <[email protected]>
Set required version of setuptools to 64, since older versions do not support PEP660.

Signed-off-by: Jonas K. <[email protected]>
Add a note how to handle the FASM warning about the missing ANTLR package.

Signed-off-by: Jonas K. <[email protected]>
@EverythingElseWasAlreadyTaken
Copy link
Collaborator Author

EverythingElseWasAlreadyTaken commented May 13, 2024

To make the warning disappear, we will need to install fasm from the source. I think just putting a note is all we need.

I've added a note in the docs and rebased to the current 2.0 changes.

@IAmMarcelJung
Copy link
Collaborator

Maybe you have to update setuptools manually and also update pip to make this work. At least this is what people with similar issues did here: pypa/setuptools#4037

Just for completion, @EverythingElseWasAlreadyTaken and I talked again about this and it was indeed the update of pip that made the global installation work.

@KelvinChung2000 KelvinChung2000 merged commit 3362417 into FPGA-Research:FABulous2.0-development Jun 10, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants