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

$PROJECT_SOURCE env variable #3781

Closed
4 tasks
kadel opened this issue Aug 19, 2020 · 8 comments · Fixed by #3979
Closed
4 tasks

$PROJECT_SOURCE env variable #3781

kadel opened this issue Aug 19, 2020 · 8 comments · Fixed by #3979
Assignees
Labels
estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects

Comments

@kadel
Copy link
Member

kadel commented Aug 19, 2020

/kind user-story

User Story

As a Devfile author, I always want to know the exact location of the project source code So that I can execute commands in a current diretory

Acceptance Criteria

  • ${PROJECT_SOURCE} env variable should be available in every container of the component so it can be used in commandLine or workingDir fields inside Devfiles
  • ${PROJECT_SOURCE} should point to a directory that contains project source code
    • if there are multiple projects in devfile ${PROJECT_SOURCE} will point to the source code of the first one
    • if there are no projects in devfile ${PROJECT_SOURCE} will be equal to the value of ${PROJECTS_ROOT}

Notes

  • ${PROJECT_SOURCE} can be different for each container because it depends on sourceMapping value and on ${PROJECTS_ROOT} value both are set per container.
  • Projects source code location inside ${PROJECTS_ROOT} depends on clonePath value (defaults to project name) (clonePath should be supported in odo #3729)

Links

/kind user-story
/priority high

@openshift-ci-robot openshift-ci-robot added kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). labels Aug 19, 2020
@girishramnani
Copy link
Contributor

so wouldn't this be blocked on #3729 @kadel ?

@kadel
Copy link
Member Author

kadel commented Aug 21, 2020

so wouldn't this be blocked on #3729 @kadel ?

not sure, it could but it also might not :-) It depends how it is handled in odo codebase.

@girishramnani girishramnani added this to For consideration in Sprint 189 via automation Aug 21, 2020
@girishramnani girishramnani added the triage/needs-information Indicates an issue needs more information in order to work on it. label Aug 24, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 189 Aug 24, 2020
@girishramnani
Copy link
Contributor

Prioritise this #3729

@girishramnani
Copy link
Contributor

$PROJECT_SOURCE is required for using parent without this env varibale parent devfiles are unusable, but we are not using it anywhere yet

@girishramnani
Copy link
Contributor

so it would be ok to implement it after GA

@girishramnani girishramnani moved this from To do to For consideration in Sprint 189 Aug 26, 2020
@girishramnani girishramnani removed this from For consideration in Sprint 189 Sep 14, 2020
@girishramnani girishramnani added this to For consideration in Sprint 190 via automation Sep 14, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 190 Sep 14, 2020
@girishramnani girishramnani added estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person triage/ready and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels Sep 17, 2020
@maysunfaisal maysunfaisal moved this from To do to In progress in Sprint 190 Sep 17, 2020
@maysunfaisal
Copy link
Contributor

maysunfaisal commented Sep 17, 2020

I had a question regarding a use case and wanted to see how we should respond to it. Consider the situation:

devfile snippet:

projects:
  - name: myproj1
    clonePath: myapp1/
    git:
      remotes:
        origin: https://github.com/odo-devfiles/springboot-ex
components:
- name: dummy
  container:
    image: quay.io/eclipse/che-java11-maven:nightly
    memoryLimit: 768Mi
    mountSources: true
    sourceMapping: /test
    command: ['tail']
    args: [ '-f', '/dev/null']
    env:
    - name: PROJECT_SOURCE
      value: /mycustom/path/

explanation:

When a user overwrites PROJECT_SOURCE via devfile env. That would mean it would take precedence over a component container's sourceMapping and project's clonePath and we would need to overwrite the container's volume mount path to this custom PROJECT_SOURCE value now for the project volume and sync to this location.

When this happens, the sourceMapping, clonePath and the existing ENV PROJECTS_ROOT(which is decided by sourceMapping and in the above snippet it would be /test, /projects otherwise), becomes pointless.

  1. Is this the correct expected behavior?
  2. what should be PROJECTS_ROOT in this case? (Note that PROJECT_SOURCE is /mycustom/path/ and PROJECTS_ROOT is /test)
  3. Ignore this 3rd ques, it is invalid (DELETED)

cc: @kadel @elsony @johnmcollier

@johnmcollier
Copy link
Member

johnmcollier commented Sep 17, 2020

@maysunfaisal So, here are my thoughts:

I actually don't think we should allow $PROJECT_SOURCE to be customized by the user for two reasons:

  • As per the schema, its value directly depends on the value of $PROJECT_ROOT, so it doesn't make sense for the two to be completely different values (say /test and /somedir)
  • The project root* is used by odo to determine where to mount the source code volume on the container and the parent folder of where to sync. So if $PROJECT_SOURCE is overridden to some other value, then it won't point to where the source code is located.

* Odo uses the following logic to determine the project root (in order):

  1. The value of sourceMapping, if set
  2. The value of the $PROJECT_ROOT env, if overridden in the container
  3. Defaults to /project otherwise

@elsony
Copy link

elsony commented Sep 17, 2020

+1 on no point for user to override the $PROJECT_SOURCE. By doing that, the user is introducing inconsistent configuration within the devfile. Source mapping and the clone path decide where to do sync code and while $PROJECT_SOURCE give you the final value on where you can find the source. If you override $PROJECT_SOURCE to a different value, the config basically say clone my source to one location and then telling the build that uses $PROJECT_SOURCE to find the source in another location.

On high level, there is no different from someone specify a clone path but then their build scripts are trying to build the source from a different location. It is an inconsistent config that the user provides. Having said that, it will be nice to flag that as a warning during devfile validation to avoid the user accidentally change the value of those variables.

If you take a look at the spec under the command section

Special variables that can be used:
$PROJECTS_ROOT: A path where projects sources are mounted

$PROJECT_SOURCE: A path to a project source ($PROJECTS_ROOT/). If there are multiple projects, this will point to the directory of the first one.

The spec already say those are special variables. We can consider to update the spec to say those are reserved variables (+ the devfile validation warning that I mentioned earlier) to make it clear.

@maysunfaisal maysunfaisal moved this from In progress to For review in Sprint 190 Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person kind/user-story An issue of user-story kind priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
No open projects
Sprint 190
  
Done
Development

Successfully merging a pull request may close this issue.

7 participants