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

install: resolve symlinks before packaging #1883

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

acomodi
Copy link
Contributor

@acomodi acomodi commented Dec 15, 2020

Signed-off-by: Alessandro Comodi acomodi@antmicro.com

Fixes #1882

@probot-autolabeler probot-autolabeler bot added the type-utils Issues is related to the scripts inside the repo. label Dec 15, 2020
@mithro mithro requested a review from litghost December 15, 2020 14:35
@@ -1470,7 +1470,7 @@ function(ADD_FPGA_TARGET)
append_file_dependency(VPR_DEPS ${ADD_FPGA_TARGET_INPUT_SDC_FILE})
get_file_location(SDC_LOCATION ${ADD_FPGA_TARGET_INPUT_SDC_FILE})
set(SDC_ARG --sdc_file ${SDC_LOCATION})
set(SDC_FILE ${SDC_LOCATION})
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work generally speaking. If the SDC is generated, this will break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in the if branch where the SDC is manually written. The case in which the SDC is auto-generated, the actual generated SDC is taken instead: https://github.com/SymbiFlow/symbiflow-arch-defs/blob/7183cb5e59d50955686c12e4b800a55fafd1fae5/common/cmake/devices.cmake#L1461-L1467

Copy link
Contributor

@litghost litghost Dec 15, 2020

Choose a reason for hiding this comment

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

Even in that cases, the user can provide a generated SDC, e.g.:

add_custom_target(
  OUTPUT my_generated.sdc
  COMMAND ... > my_generated.sdc
  )
add_file_target(FILE my_generated.sdc GENERATED)
add_fpga_target(
 ...
 SDC_FILE my_generated.sdc
 )

The new code will break in this case.

Copy link
Contributor Author

@acomodi acomodi Dec 17, 2020

Choose a reason for hiding this comment

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

I have changed approach and now the symlinks are resolved before packaging tests into the tarball

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@probot-autolabeler probot-autolabeler bot added the type-infra Issues related to infra like travis. label Dec 17, 2020
@acomodi acomodi changed the title install: if SDC is manually written install source, not symlink install: resolve symlinks before packaging Dec 17, 2020
@litghost litghost merged commit af5913d into f4pga:master Dec 17, 2020
@acomodi acomodi deleted the fix-sdc-install branch March 23, 2021 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-infra Issues related to infra like travis. type-utils Issues is related to the scripts inside the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad links in benchmarks tarball from CI
3 participants