-
Notifications
You must be signed in to change notification settings - Fork 56
Migrating native sim from GCC to Clang. #553
Conversation
…Debian:buster Docker image).
|
Please review. |
images/iqsharp-base/Dockerfile
Outdated
| # Note that we install them here to minimize the number | ||
| # of layers. | ||
| libgomp1 \ | ||
| libomp-7-dev \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed in the iqsharp-base Dockerfile? That indicates to me that even with static linking, there's a new requirement passed onto users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static linking attempt has failed. I failed to find the libomp.a static library, it wasn't installed as a part of libomp package (I didn't try to build libomp.a from sources). Googling out didn't help, @idavis, correct me if I'm wrong.
Also, IIRC, there was some hypothetical risk with licensing, if we provide libomp together with our native simulator. But installing the libomp had no any licensing issues.
Why is this needed in the iqsharp-base Dockerfile?
Because the native simulator dynamic library is built on ubuntu-latest, where libomp is already installed, but the iqsharp-base Docker image (that uses the native simulator dynamic library) is run on Debian Buster (Debian 10), where the libomp is not installed. So I added the installation of libomp on Debian Buster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kuzminrobin can we try libomp5-7? That should just be the libomp runtime.
cgranade
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking as request changes to make sure that we don't add additional requirements to users without double-checking first.
|
Approved, but we should be sure to document this change; users who work directly on Linux or in custom containers may need to update from their previous dependency. |
|
@cgranade, any direct links to the documentation files that I need to update are more than welcome. If you mean just the QDK Release Notes (of January 2022 release) then feel free to reply with "relnotes". |
I don't have any off the top of my head, sorry, but some of the install docs used to mention the libgomp1 dependency; searching for if there is still such a mention may be helpful, perhaps? |
|
A set of 3 PRs: Q#RT, QDK, IQ# repos (see also quantum-docs PR).
A set of the latter 2 are expected to require a branch-specific e2e build or a merge of the former one first.