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

wip: packaging scripts #699

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

aws-nslick
Copy link
Contributor

This generates an srpm (bundled src + spec) and a dsc bundle. It's completely
versionless and takes in only an ish to generate a package bundle for.

@@ -6,7 +6,7 @@
#

# Initialization
AC_INIT([aws-ofi-nccl], [GitHub-dev], [[email protected]], , [http://github.com/aws/aws-ofi-nccl])
AC_INIT([aws-ofi-nccl], [1.13.0-pre], [[email protected]], , [http://github.com/aws/aws-ofi-nccl])
Copy link
Contributor

Choose a reason for hiding this comment

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

Nack until this is on a release branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How should versions be assigned to packages for development commits?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think they should be packaged, but we definitely shouldn't have a version string that's valid on master, because that has bit us in the ass in the past, and will again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • We cannot package it without a version.

  • We want to test that packaging definitions do not break from any given change.

How should versions be assigned to packages for development commits?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not want "next" to be the version in master. If you want to set it to 999.99 or something, that's fine, I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I would really like to do is just do our branching and tagging the way that git expects, such that git describe yields a reasonable version string on every commit going forward.


override_dh_shlibdeps:
echo "libfabric 1 libfabric1-aws (>= 1.22.0amzn2.0)" >> debian/shlibs.local
echo "libcudart 12 $(CUDA_LIB_PROVIDER)" >> debian/shlibs.local
Copy link
Contributor

Choose a reason for hiding this comment

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

While technically correct, we actually need to not have a dependency on libcudart's package. If the runscript installation method is used, dpkg isn't going to know about the libcudart.so file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why bother with debian native packaging if we're not going to actually declare our dependencies?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no win here. But this is what we're doing with Libfabric and what we need to do here. Stop being so pedantic and look at the customer picture. You can't require them to use the native packaging installation, when most use the run script today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I mean why package with debscripts instead of just using something like fpm or checkinstall

Copy link
Contributor

Choose a reason for hiding this comment

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

because that's how the efa installer works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way that debian native packaging works is that if a library links another library, every DT_NEEDED entry on the binary needs to be resolved from some package that ships a file like:

$ dpkg-query -c libhwloc15               
/var/lib/dpkg/info/libhwloc15:amd64.md5sums
/var/lib/dpkg/info/libhwloc15:amd64.shlibs
/var/lib/dpkg/info/libhwloc15:amd64.triggers
$ cat "/var/lib/dpkg/info/libhwloc15:amd64.shlibs"

libhwloc 15 libhwloc15 (>= 2.10.0)

One can disable this completely on specific libraries with the dh_shlibdeps -X option, but it means that we then need to manually maintain dependency information for libfabric and hwloc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd also note that libfabric-aws does not do this correctly and/or doesn't ship a symbols file.

Copy link
Contributor Author

@aws-nslick aws-nslick Nov 7, 2024

Choose a reason for hiding this comment

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

Likely this is due to installing under /opt:

The package should install the shared libraries under their normal names. For example, the libgdbm3 package should install libgdbm.so.3.0.0 as /usr/lib/libgdbm.so.3.0.0. The files should not be renamed or re-linked by any prerm or postrm scripts; dpkg will take care of renaming things safely without affecting running programs, and attempts to interfere with this are likely to lead to problems.

from https://www.debian.org/doc/debian-policy/ch-sharedlibs.html

contrib/scripts/generate_debian_changelog.sh Show resolved Hide resolved
@aws-nslick aws-nslick added this to the 2.0 milestone Nov 14, 2024
To support sane packaging changelog generation from github releases, add
a mailmap file that's capable of ensuring that git-log shows reasonable
email addresses. Also add a gitattributes file so that `git archive`
contents are cleaner.

Signed-off-by: Nicholas Sielicki <[email protected]>
whenever we cut a release branch, two commits should be made:

1. targeting master, bumping the version to maj.<min+1>.0-pre

2. targeting the release branch, stabilizing maj.min.0-pre into
   maj.min.0

This makes it much easier to package.
usage: ./contrib/scripts/generate_source_package.sh <tag>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants