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

Add process specific working directories #1427

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

imnitishng
Copy link
Contributor

@imnitishng imnitishng commented Apr 24, 2022

Summary

  • Show process' working directory from the WorkingDirectory param in metadata label
  • If working directory from label is empty then use Image's inspect to figure out the working directory (stored as WorkingDir)
    This PR depends on Add WorkingDir() to return image working directory imgutil#141, I did not find a way to expose the working directory so I had to go for this change, let me know if there's another way to handle the problem.
    Moreover, I had to update a bunch of dependencies to actually get the newly added WorkingDirectory param inside of process, which means go.mod and go.sum are modified.
    I think we'll need to update the PR again with the new imgutil release if we decide to merge both PRs and go this route, if not then I'd be happy to work on the feedback.
    Tests will be failing until we merge the PR for imgutil

Output

Before

After

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

#1422

Resolves #1422

@imnitishng imnitishng requested a review from a team as a code owner April 24, 2022 20:06
@github-actions github-actions bot added type/chore Issue that requests non-user facing changes. type/enhancement Issue that requests a new feature or improvement. labels Apr 24, 2022
@github-actions github-actions bot added this to the 0.26.0 milestone Apr 24, 2022
@imnitishng imnitishng force-pushed the 25-04-process-working-dirs branch 8 times, most recently from 0a006be to 4e0d03f Compare April 26, 2022 16:57
@imnitishng
Copy link
Contributor Author

Updating the PR soon, with passing test 😬

- Show process' working directory from the `WorkingDirectory`
  param in metadata label
- If working directory from label is empty then use Image's
  inspect to figure out the working directory (stored as WorkingDir)

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
@imnitishng
Copy link
Contributor Author

imnitishng commented Apr 29, 2022

I think we should be good to go once buildpacks/imgutil#144 is merged

@imnitishng imnitishng force-pushed the 25-04-process-working-dirs branch 4 times, most recently from 35e469d to e4e8e58 Compare April 29, 2022 18:31
@imnitishng
Copy link
Contributor Author

imnitishng commented Apr 29, 2022

Hey @jromero, I am having issues with the spacing for WORK DIR, for windows and linux.
The spacing for both the platforms is a bit different, but the template is same.
Should I create new template files .txt for windows and linux? Or should we modify the test in some other way?
Once this is fixed, we should be good to go.

@jromero
Copy link
Member

jromero commented Apr 29, 2022

Hey @jromero, I am having issues with the spacing for WORK DIR, for windows and linux. The spacing for both the platforms is a bit different, but the template is same. Should I create new template files .txt for windows and linux? Or should we modify the test in some other way? Once this is fixed, we should be good to go.

image

I can see the issue in Windows. It looks as though because WORK DIR comes after ARGS and on windows, args are pushing the column further to the right.

Based on our conversation on #buildpacks-platform. I'd be comfortable removing the acceptance test for human-readable output comparison.

In otherwords, deleting these lines (L976-L980, L1627-L1631) and any associated resources from pack_fixtures.

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
@jromero jromero removed the type/chore Issue that requests non-user facing changes. label Apr 29, 2022
@jromero jromero merged commit bcd742d into buildpacks:main Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display process-specific working directories when available
2 participants