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

Use escaped id when creating buildpack tar file #112

Merged
merged 1 commit into from Mar 26, 2019
Merged

Conversation

jkutner
Copy link
Member

@jkutner jkutner commented Mar 21, 2019

This fixes the case when the buildpack ID contains a /, which is allowed.

I'm also unsure if using the raw version string is acceptable. We should probably hash the whole buildpack name/id/version since this is never user facing.

@jkutner jkutner requested a review from ekcasey March 21, 2019 15:25
@jkutner jkutner added the type/bug Issue that reports an unexpected behaviour. label Mar 21, 2019
@jkutner jkutner added this to In Progress in Planning Board Mar 21, 2019
@jkutner jkutner moved this from In Progress to Release v0.1.0 in Planning Board Mar 21, 2019
@jkutner jkutner moved this from Release v0.1.0 to In Progress in Planning Board Mar 21, 2019
Copy link
Member

@ekcasey ekcasey left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for the fix. I am happy to merge once the conflicts are resolved.

I agree that we should probably escape versions. However, I believe that would require lifecycle changes first. The buildpack directory names shouldn't be "user facing" but currently the the BuildpackMap used for detection in lifecycle assumes that the version directory will match the version provided in group. We should change that to use the version in buildpack.toml instead of the directory name.

https://github.com/buildpack/lifecycle/blob/7c4df24760f6778e2cfa9bc862a72a4d2b372019/map.go#L30-L35

@jkutner jkutner force-pushed the fix-tmp-dir-name branch 2 times, most recently from 056de37 to 6654079 Compare March 25, 2019 21:59
Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@ekcasey ekcasey merged commit 794b050 into master Mar 26, 2019
@ekcasey ekcasey deleted the fix-tmp-dir-name branch March 26, 2019 12:23
@ekcasey ekcasey moved this from In Progress to Done in Planning Board Mar 26, 2019
@sclevine sclevine moved this from Done to Release v0.1.0 in Planning Board Apr 3, 2019
@jromero jromero added this to the 0.1.0 milestone Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Issue that reports an unexpected behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants