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

Docker: use correct MAVEN_CONFIG #182

Merged
merged 1 commit into from
May 30, 2018
Merged

Conversation

jaypea
Copy link
Contributor

@jaypea jaypea commented May 30, 2018

the variable wasn't used correctly. i always fell back to use . as homedir.

the variable wasn't used correctly. i always fell back to use `.` as homedir.
Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

The fix looks good to me (although I didn't have a chance to test it locally)

@wing328 wing328 added this to the 3.0.0 milestone May 30, 2018
@jaypea
Copy link
Contributor Author

jaypea commented May 30, 2018

this is a quick test in your console:

$ MAVEN_CONFIG=/some/dir
$ dirname MAVEN_CONFIG
.
$ dirname $MAVEN_CONFIG
/some

in the first case, dirname is called with string 'MAVEN_CONFIG'
in the second run its called with the contents of the variable $MAVEN_CONFIG

@jimschubert
Copy link
Member

jimschubert commented May 30, 2018

@jaypea this is obviously a correct fix for the script as it is written, and I'm going to merge it.

I was just wondering, what is your use case where you discovered this?

If you build the Dockerfile directly, it looks like this would result in the setting at /opt/openapi-generator. It seems like run-in-docker.sh sets this to /var/maven/.m2, then maps to your local .m2 repo. I have concerns that this change may cause host m2 repository entries to have root permissions. Sharing your use case would help me evaluate the impact, and whether we might need to update the script(s).

@jimschubert jimschubert merged commit 7dfd940 into OpenAPITools:master May 30, 2018
@jimschubert
Copy link
Member

oh, at first glance I missed that we are in fact setting user/group in run-in-docker.sh. I'm still interested in the use case, though.

@jaypea jaypea deleted the patch-1 branch May 31, 2018 08:14
@jaypea
Copy link
Contributor Author

jaypea commented May 31, 2018

@jimschubert I was using run-in-docker.sh to build a branch in my generator fork and to create a client sdk from this branch.
In order to automate this further, I tried to run this inside of circleci. mounting volumes doesn't work there so i had to cp the repository cache inside of a docker volume. This lead me to this issue.

this is the script i am using in circleci now:

// .circleci/config.yml

version: 2
jobs:
  build:
    working_directory: ~/nyslough-client-sdk
    docker:
      - image: circleci/node:8-stretch

    steps:
      - checkout
      - setup_remote_docker:
          docker_layer_caching: true

      - restore_cache:
          keys:
          - codegen

      - run: |
          set -x
          API_URL=${1:-https://url-to/v2/api-docs}
            mkdir -p build
            cd build
            
            test -d swagger-codegen || git clone https://github.com/jaypea/swagger-codegen
            
            cd swagger-codegen
            git checkout js-flow-typed
            git pull
            wget $API_URL -O openapi.json
            
            docker create -v /gen -v /var/maven/.m2/repository --name artifacts alpine:3.4 /bin/true
            
            maven_cache_repo="${HOME}/.m2/repository"
            
            mkdir -p "${maven_cache_repo}"
            mkdir -p ${PWD}/output
            
            docker cp ${PWD}/. artifacts:/gen
            docker cp ${maven_cache_repo}/. artifacts:/var/maven/.m2/repository
            
            docker run --name codegen \
                    -w /gen \
                    -e GEN_DIR=/gen \
                    -e MAVEN_CONFIG=/var/maven/.m2 \
                    --volumes-from artifacts:rw \
                    --entrypoint /gen/docker-entrypoint.sh \
                    maven:3-jdk-7 generate -i openapi.json -l javascript-flowtyped -o output
            
            cd ../../
            docker cp codegen:/gen/output/. ./
            docker cp codegen:/gen/. ./build/swagger-codegen/
            docker cp codegen:/var/maven/.m2/repository/. ${maven_cache_repo}
            docker rm codegen
            docker rm artifacts
            
            npm install
            NODE_ENV=production npm run build

            git commit -am 'automatic update of client sdk'

      - save_cache:
          paths:
            - node_modules
            - build
            - ~/.m2/repository
          key: codegen

@jaypea
Copy link
Contributor Author

jaypea commented May 31, 2018

Though, i had to remove the userid setting in docker run because i didn't want to change the docker-entrypoint.sh to this extend. I would rather chown the mounted repository, currently its owned by root in the container.

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

Successfully merging this pull request may close these issues.

None yet

3 participants