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

[Overview] Dependency Management RFC #204

Merged
merged 4 commits into from
Aug 8, 2022

Conversation

sophiewigmore
Copy link
Member

@sophiewigmore sophiewigmore commented Jun 1, 2022

Summary

Readable

Overview RFC of the three-phased approach to changing our dependency management system.

Use Cases

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@sophiewigmore sophiewigmore changed the title Add top-level dependency management RFC [Overview] Dependency Management RFC Jun 1, 2022
@sophiewigmore sophiewigmore marked this pull request as ready for review June 6, 2022 15:41
@sophiewigmore sophiewigmore requested a review from a team as a code owner June 6, 2022 15:41
@sophiewigmore sophiewigmore force-pushed the dependency-management-top-level branch from 3e3c205 to 0493d63 Compare June 6, 2022 15:46
Comment on lines 115 to 116
only Ubuntu 18.04 dependencies, which is what we have now due to the way
dependencies are compiled. While some dependencies may still need to be
Copy link
Member

Choose a reason for hiding this comment

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

Is this really a compilation problem or is it a metadata problem?

For example our compilation of node runs fine on jammy apparently

docker run -it -v /tmp/paketo-node:/node ubuntu:jammy bash
root@eaa064b73682:/# cd node
root@eaa064b73682:/node# tar xzf node_v14.19.2_linux_x64_bionic_3f263e3e.tgz
root@eaa064b73682:/node# bin/node --help
Usage: node [options] [ script.js ] [arguments]
       node inspect [options] [ script.js | host:port ] [arguments]

And indeed dynamically links against the same (presumably stable) set of glilbc APIs as the upstream distribution

cnb@0e9092f382aa:/workspace$ ldd /layers/paketo-buildpacks_node-engine/node/bin/node
	linux-vdso.so.1 (0x00007ffd61cd1000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f4729087000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f4728cfe000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f4728960000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f4728748000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f4728529000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f4728138000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f472dc00000)

In cases where the results of compilation differ between stacks (e.g. nodejs for alpine or nginx probably) we will still need multiple version per stack even if we fetch binaries from from upstream.

Not suggesting that we shouldn't move away from recompiling, but that in some cases we could add support for more stacks with our current set of deps.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea you're right. It won't be uniform across the board, some dependencies will just work as they are now, but it'll be up to maintainers to figure that out per dependency/stack offering and make the choice that works for that dependency

Comment on lines 101 to 102
the Motivation section of the RFC. Due to the complex nature of the changes,
the proposals details will be split into separate RFCs.
Copy link
Member

Choose a reason for hiding this comment

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

Can we approve one of these phases without the others? Seems like we can't approve the overview without approving the phases at least. In which case, I am not sure what gain from splitting the RFC up. I could definitely see us tracking implementation in three phases but maybe it would be better to have the entire plan (including the three phases) in a single document so we can approve it all in one go and reference the details in one place.

Copy link
Member Author

Choose a reason for hiding this comment

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

The phases are pretty dependent on each other, but I think there are some details that are independent. We could agree on Phase 1 (buildpack-related changes) and get that approved, and then go back to the Phase 2 RFC and hash out the details afterwards. The separation of RFCs is tied to the phased implementation, but I do think it makes the content more digestible by separating concerns. It's a lot of content to put into one RFC in my opinion.

Copy link
Member Author

@sophiewigmore sophiewigmore Jun 14, 2022

Choose a reason for hiding this comment

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

It makes it easier for me as the RFC author to have a sort of separation of concerns, but there is some overlap that may make it confusing for others. My intention with the phases was to make it clearer!

@fg-j
Copy link

fg-j commented Jul 19, 2022

Meeting today to discuss.
Details: https://paketobuildpacks.slack.com/archives/C011S6EL49L/p1657818502769609

@joshuatcasey
Copy link
Contributor

joshuatcasey commented Jul 26, 2022

2022-07-26 Working Group notes:

@ryanmoran encourages merging, we have clarity around the broad strokes in these dependency RFCs and we can go ahead and merge, recommends not waiting for the stack removal from https://github.com/buildpacks/rfcs/blob/main/text/0096-remove-stacks-mixins.md. We should be able to migrate from the current dependency metadata to the "new" metadata easily enough.

@sophiewigmore sophiewigmore merged commit 44182c7 into main Aug 8, 2022
@sophiewigmore sophiewigmore deleted the dependency-management-top-level branch August 8, 2022 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants