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

Remove PDK build targets from Makefile #563

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

donn
Copy link
Member

@donn donn commented Feb 7, 2025

As discussed--

  • pdk can now only be fetched with volare -- users who want to build their own PDK should know what they're doing
  • volare, rst_include now share a venv
  • venv no longer needlessly recreated every single time volare is installed
  • venv added to gitignore
  • check-python reimplemented

- pdk can now only be fetched with volare -- users who want to build their own PDK should know what they're doing
- volare, rst_include now share a venv
- venv no longer needlessly recreated every single time volare is installed
- venv added to gitignore
- check-python reimplemented
@donn donn force-pushed the remove_custom_opdks_build branch from 760cc96 to e9201f6 Compare February 7, 2025 17:37
@DavidRLindley DavidRLindley changed the base branch from main to ci2504-DEV February 7, 2025 17:57
@DavidRLindley DavidRLindley merged commit 38f2bbf into ci2504-DEV Feb 7, 2025
3 checks passed
@d-m-bailey
Copy link
Contributor

@donn thanks for the quick response.

Some comments.
1.

	@$(PYTHON_BIN) -c "import sys; assert sys.version_info >= (3, 6), 'Python version less than 3.6'"
	@echo "Python >=3.8 found."

Python 3.6 and 3.7 will pass the assertion, but the message will be >= 3.8, won't it?

  1. Can we keep the deprecated targets, but add messages to the ones to give the user some idea of what changed?
    For example, with skywater-pdk
skywater-pdk: 
	@echo "The skywater-pdk target has been deprecated. Please use make pdk-with-volare."
	exit 1
  1. This is pedantic, but since the check-env dependency is included in pdk-with-volare, it's probably not needed on the pdk target too.

  2. Since there are changes to the README.rst target, can you go ahead and make the change to remove openlane/README.rst too?

@d-m-bailey
Copy link
Contributor

@DavidRLindley @donn Since this was just merged, I'll make the above changes along with the my changes later.

The python message should be, >=3.6 right?

@donn
Copy link
Member Author

donn commented Feb 7, 2025

@d-m-bailey Yes, oops. Thank you.

I initially wanted it to make it 3.8 but I just know someone on CentOS 7 will manifest from thin air.

@d-m-bailey
Copy link
Contributor

@donn There may be a problem with the pdk-with-volare dependency requirements.txt.

It works when executed from caravel directory, but when executed from the caravel_user_project directory, I get

make[1]: *** No rule to make target 'requirements.txt', needed by 'venv/manifest.txt'.  Stop.

Would changing

venv/manifest.txt: ./requirements.txt

to

venv/manifest.txt: $(CARAVEL_ROOT)/requirements.txt

be sufficient?

@donn
Copy link
Member Author

donn commented Feb 14, 2025

I believe so.

@d-m-bailey
Copy link
Contributor

@donn Thanks. There's a place in the recipe that needs to be changed also. I'll make the change.

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