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

Fix sources clone #373

Merged
merged 2 commits into from
May 11, 2023

Conversation

kareefardi
Copy link
Contributor

@kareefardi kareefardi commented May 10, 2023

  • Add ; \ to chain commands to be able to reuse the shell variable created earlier. Each line of a make recipe is a separate shell instance.
  • Escape ${FD_PV_COMMIT}, and similar variables, to use the shell variable created in the recipe
  • In download.sh, if git clone --branch fails, attempt to git clone and checkout - for commits.

Copy link
Owner

@RTimothyEdwards RTimothyEdwards left a comment

Choose a reason for hiding this comment

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

Ah, so there was a lot more than just the space around "=".

@RTimothyEdwards
Copy link
Owner

Pulled and merged.

@RTimothyEdwards RTimothyEdwards merged commit db529a7 into RTimothyEdwards:master May 11, 2023
donn added a commit to donn/open_pdks that referenced this pull request May 8, 2024
RTimothyEdwards#373, specifically commit `db529a71335176db64b18b98228fdd7f03a8c24b` fixed an issue where the shell commands inside `gf180mcu/Makefile.in` were not properly chained for some make productions, causing the branch commit to be dropped, however, the same issue persisted for `sky130`. This caused builds to be non-reproducible for `sky130` where the Makefile would *always* grab the latest for each repo. The result was `--with-reference` never worked for sky130.

This also adds an additional print to `scripts/download.sh` just to verify the commit is correctly propagated to the `git clone` commands.
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.

2 participants