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

Dockerizing OpenSTA #14

Merged
merged 5 commits into from Jan 20, 2019

Conversation

Projects
None yet
2 participants
@abdelrahmanhosny
Copy link
Contributor

abdelrahmanhosny commented Jan 9, 2019

This pull request proposes packaging OpenSTA in a Docker image. Users don't have to run through lengthy and buggy installation steps. The Docker image is available on Docker Hub (openroad/opensta).

After installing Docker, run OpenSTA using docker run -it -v $(pwd):/input openroad/opensta where -v $(pwd):/input mounts the current directory to a directory inside the Docker container called input, where the input files reside.

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 10, 2019

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 16, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 16, 2019

I'm working on another version now that:

  1. Uses dockerfile COPY command to add files from the repo. This will help make versions of the Docker image according to releases on the repo here.
  2. Uses cmake instead of autoconf. It's just taking some time as there are errors when including cudd.h which I'm still trying to solve.
@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 16, 2019

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 16, 2019

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 17, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 17, 2019

Ok. I'm going to rebase my branch on this repo branch and edit the Docker file accordingly.
CMD and ENTRYPOINT can be used interchangeably in this context. I will do the testing as well to make sure it works properly.
Expect an update by late tomorrow.

abdelrahmanhosny added some commits Jan 17, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 17, 2019

So, I polished the Dockerfile and updated the README file to point users to how to run the Docker image. I also pushed an image of OpenSTA to Docker Hub at https://hub.docker.com/r/openroad/opensta

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 18, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 18, 2019

I updated the link. Here it is: https://hub.docker.com/r/openroad/opensta

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 18, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 18, 2019

According to the official Docker documentation, it says:
For interactive processes (like a shell), you must use -i -t together in order to allocate a tty for the container process. -i -t is often written -it as you’ll see in later examples.

I know it works with only -i flag, but it doesn't show the % sign for the beginning of writing the command. I am not sure what other side effect we might have leaving out the -t flag.

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 18, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 19, 2019

Hi James,
Is there any more work to be done on this pull request in order to be merged and closed? :)

@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 19, 2019

@abdelrahmanhosny

This comment has been minimized.

Copy link
Contributor

abdelrahmanhosny commented Jan 19, 2019

So, can you confirm the following changes to be committed to this merge request?

  1. Move the Docker section to the install file. Confirm?
  2. I can't see the recent README edit that only has SPEF in it. Can you point me out to the line number?
@jjcherry56

This comment has been minimized.

Copy link
Collaborator

jjcherry56 commented Jan 20, 2019

I guess I hadn't pushed it. You should see it now.

@jjcherry56 jjcherry56 merged commit b700eca into abk-openroad:master Jan 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment