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

Added support for overriding installation path via DESTDIR #306

Merged
merged 1 commit into from
May 13, 2022

Conversation

mkurc-ant
Copy link
Collaborator

This PR adds support for the DESTDIR Makefile variable which allows to override the default plugin & data installation path provided by yosys-config.

… Makefile variable

Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

EXTRA_FLAGS ?=

ifdef DESTDIR
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, the default value is empty, you can just prepend it where needed. Also, it's better hygiene to just put it in the commands that write to the filesystem, instead of changing the value of PLUGINS_DIR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that I can simply prepend DESTDIR to the path provided by yosys-config --destdir command. The latter is an absolute path. I have to change DATA_DIR because it is referenced in plugin-specific Makefiles to know where to put additional data files.

@alaindargelas
Copy link

Can we get this merged? The default is not correct for me.

@tgorochowik tgorochowik merged commit aadd173 into chipsalliance:main May 13, 2022
@tgorochowik tgorochowik deleted the install-destdir branch May 13, 2022 07:56
mglb pushed a commit to antmicro/yosys-f4pga-plugins that referenced this pull request Apr 3, 2023
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.

None yet

5 participants