search

The-OpenROAD-Project / OpenSTA

OpenSTA engine
GNU General Public License v3.0
399 stars 172 forks source link

Dockerizing OpenSTA #14

Closed abdelrahmanhosny closed 5 years ago

abdelrahmanhosny commented 5 years ago

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 commented 5 years ago

you should use cmake instead of autotools, since the autotools is likely to disappear soon

On Wednesday, January 9, 2019, Abdelrahman notifications@github.com wrote:

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.

You can view, comment on, or merge this pull request online at:

https://github.com/abk-openroad/OpenSTA/pull/14 Commit Summary

  • dockerizing OpenSTA

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lRD2Zuk_kge-VV8ea0iaGhgK79dVks5vBn-MgaJpZM4Z4hVJ .

jjcherry56 commented 5 years ago

It is also broken because it pulls from your private repo instead of the github repo.

On Thursday, January 10, 2019, James Cherry cherry@parallaxsw.com wrote:

you should use cmake instead of autotools, since the autotools is likely to disappear soon

On Wednesday, January 9, 2019, Abdelrahman notifications@github.com wrote:

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.

You can view, comment on, or merge this pull request online at:

https://github.com/abk-openroad/OpenSTA/pull/14 Commit Summary

  • dockerizing OpenSTA

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lRD2Zuk_kge-VV8ea0iaGhgK79dVks5vBn-MgaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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 commented 5 years ago

When I build CUDD I don't use any configure args. I don't bother with 'make check'. I don't see 'make install' for the results; make a cudd dir someplace and set path prefix there and point the sta build to it. Like this:

cd $HOME/cudd-3.0.0 mkdir $HOME/cudd ./configure --prefix $HOME/cudd make

and then for OpenSTA build .. -DCUDD=$HOME/cudd

On Wednesday, January 16, 2019, Abdelrahman notifications@github.com wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-454886640, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lavsjITh-1h6qU8uXRM3JuJeLwPBks5vD259gaJpZM4Z4hVJ .

jjcherry56 commented 5 years ago

try again

cd $HOME/cudd-3.0.0 mkdir $HOME/cudd ./configure --prefix $HOME/cudd make make install

and then for OpenSTA cmake .. -DCUDD=$HOME/cudd Reply Forward

On Wednesday, January 16, 2019, James Cherry cherry@parallaxsw.com wrote:

When I build CUDD I don't use any configure args. I don't bother with 'make check'. I don't see 'make install' for the results; make a cudd dir someplace and set path prefix there and point the sta build to it. Like this:

cd $HOME/cudd-3.0.0 mkdir $HOME/cudd ./configure --prefix $HOME/cudd make

and then for OpenSTA build .. -DCUDD=$HOME/cudd

On Wednesday, January 16, 2019, Abdelrahman notifications@github.com wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-454886640, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lavsjITh-1h6qU8uXRM3JuJeLwPBks5vD259gaJpZM4Z4hVJ .

jjcherry56 commented 5 years ago

I pushed an update that has a Dockerfile based on yours that successfully builds OpenSTA. I have never used docker before and don't see how to actually run the image. I think it should use ENTRYPOINT instead of CMD to pass the sta command file in as an arg to docker run but it doesn't seem to work properly. It looks like stdout goes off to never never land.

On Wednesday, January 16, 2019, James Cherry cherry@parallaxsw.com wrote:

try again

cd $HOME/cudd-3.0.0 mkdir $HOME/cudd ./configure --prefix $HOME/cudd make make install

and then for OpenSTA cmake .. -DCUDD=$HOME/cudd Reply Forward

On Wednesday, January 16, 2019, James Cherry cherry@parallaxsw.com wrote:

When I build CUDD I don't use any configure args. I don't bother with 'make check'. I don't see 'make install' for the results; make a cudd dir someplace and set path prefix there and point the sta build to it. Like this:

cd $HOME/cudd-3.0.0 mkdir $HOME/cudd ./configure --prefix $HOME/cudd make

and then for OpenSTA build .. -DCUDD=$HOME/cudd

On Wednesday, January 16, 2019, Abdelrahman notifications@github.com wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-454886640, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lavsjITh-1h6qU8uXRM3JuJeLwPBks5vD259gaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

that link 404'd for me

On Thursday, January 17, 2019, Abdelrahman notifications@github.com wrote:

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://cloud.docker.com/u/openroad/repository/docker/ openroad/opensta

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-455356408, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8ldzn9S4TGHToA26J8dEkxwcOeWd0ks5vEPmMgaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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

jjcherry56 commented 5 years ago

I got it to run as advertised. One small nit; the run command arg -it should be -i for interactive in README

On Thursday, January 17, 2019, Abdelrahman notifications@github.com wrote:

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

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-455456733, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lSVQ5l69Jq2EEYojKOf3WubVmVtYks5vEXyfgaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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 commented 5 years ago

got it. thanks.

On Friday, January 18, 2019, Abdelrahman notifications@github.com wrote:

According to the official Docker documentation https://docs.docker.com/engine/reference/run/#foreground, 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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-455659161, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8lTv-2BjScVGyDmDyhPg_ZZc4O3tsks5vEh8wgaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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

jjcherry56 commented 5 years ago

I don't feel comfortable with the README being so docker centric. I think that INSTALL may be a better home for it. The README.md line that has RSPF/DSPF/SPEF should be updated to SPEF to stay in sync with a recent README edit.

On Friday, January 18, 2019, Abdelrahman notifications@github.com wrote:

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

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/abk-openroad/OpenSTA/pull/14#issuecomment-455752513, or mute the thread https://github.com/notifications/unsubscribe-auth/AhI8leSqclQl5qETPlF-hehH6oHUi5Zqks5vErLogaJpZM4Z4hVJ .

abdelrahmanhosny commented 5 years ago

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 commented 5 years ago

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