Skip to content

Conversation

@kartg
Copy link
Contributor

@kartg kartg commented Mar 7, 2025

Proposed Changes

This change adds a source-bundle build target to the RabbitMQ Makefile. This target is identical to the existing source-dist target but it also allows for packaging and testing of the source archive.

Code duplication is avoided by moving the common code for both targets to a generic function, which is responsible for creating the actual Makefile targets. The differing properties (like rsync flags) are passed in as function arguments.

This is a re-implementation of the change in #13385. Additional testing has been done to verify that existing targets do not change/break.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)
  • Build system and/or CI

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
If you're unsure about any of them, don't hesitate to ask on the mailing list.
We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally with my changes
  • If relevant, I have added necessary documentation to https://github.com/rabbitmq/rabbitmq-website
  • If relevant, I have added this change to the first version(s) in release-notes that I expect to introduce it

Further Comments

This PR consists of two commits - the earlier commit adds the source-bundle in a straightforward manner by duplicating the logic for source-dist and simply modifying the rsync flags. The second commit refactors common code to a generic function, as described above.

kartg added 2 commits March 7, 2025 10:38
…ist" target

The main difference is that the "bundle" target does NOT exclude packaging and testing directories, which enables packaging and testing of the source archive.

Signed-off-by: Kartik Ganesh <[email protected]>
This is done by introducing a generic function that holds the common code, which then creates these two targets. The differing properties (like rsync flags) are passed in as function arguments.

Signed-off-by: Kartik Ganesh <[email protected]>
@mergify mergify bot added the make label Mar 7, 2025
@kartg
Copy link
Contributor Author

kartg commented Mar 7, 2025

Test process details

Setup

First, I ran the source-dist target on the main branch and saved the output for comparison:

% mkdir ../tmp_main/

% make source-dist RABBITMQ_VERSION=4.99.99-xyz.1234abcd PRODUCT_VERSION=4.99.99-xyz.1234abcd PROJECT_VERSION=4.99.99-xyz.1234abcd
GEN  [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd
...
 GEN    [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.manifest
 GEN    [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.tar.xz

% cp -r PACKAGES/* ../tmp_main/

Next, I switched to the feature branch and executed the source-dist and source-bundle Makefile targets

% make clean distclean
 GEN    coverdata-clean
 GEN    clean-dist
 GEN    clean-rabbitmq-server
 GEN    clean-rabbitmq-server-bundle
 GEN    distclean-tmp
 GEN    distclean-kerl
 GEN    distclean-beam-cache
 GEN    distclean-ct
 GEN    distclean-plt
 GEN    distclean-edoc
 GEN    distclean-escript
 GEN    cover-report-clean
 GEN    distclean-packages

% make source-dist source-bundle RABBITMQ_VERSION=4.99.99-xyz.1234abcd PRODUCT_VERSION=4.99.99-xyz.1234abcd PROJECT_VERSION=4.99.99-xyz.1234abcd
GEN   [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd
...

% ls PACKAGES/
rabbitmq-server-4.99.99-xyz.1234abcd/                 rabbitmq-server-4.99.99-xyz.1234abcd.tar.xz           rabbitmq-server-bundle-4.99.99-xyz.1234abcd.manifest
rabbitmq-server-4.99.99-xyz.1234abcd.manifest         rabbitmq-server-bundle-4.99.99-xyz.1234abcd/          rabbitmq-server-bundle-4.99.99-xyz.1234abcd.tar.xz

source-dist verification

The following commands produce no diff/output, verifying that the artifacts produced by the source-dist target in the feature branch are identical to that of main:

% diff ../tmp_main/rabbitmq-server-4.99.99-xyz.1234abcd.manifest PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.manifest
<no output>

% diff <(tar -tf ../tmp_main/rabbitmq-server-4.99.99-xyz.1234abcd.tar.xz) <(tar -tf PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.tar.xz)
<no output>

% diff <(ls ../tmp_main/rabbitmq-server-4.99.99-xyz.1234abcd/) <(ls PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd/)
<no output>

source-bundle compared to source-dist

The following commands verify that the only difference between the output of source-dist and source-bundle is the inclusion of the packaging and test directories:

% comm -13 PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.manifest <(sed 's/-bundle//g' PACKAGES/rabbitmq-server-bundle-4.99.99-xyz.1234abcd.manifest) | grep -v "/packaging" | grep -v "/test"
<no output>

% comm -13 <(tar -tf PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd.tar.xz) <(tar -tf PACKAGES/rabbitmq-server-bundle-4.99.99-xyz.1234abcd.tar.xz | sed 's/-bundle//g') | grep -v "/packaging" | grep -v "/test"
<no output>

% diff <(ls ../tmp_main/rabbitmq-server-4.99.99-xyz.1234abcd/) <(ls PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd/)
<no output>

% comm -13 <(ls PACKAGES/rabbitmq-server-4.99.99-xyz.1234abcd/) <(ls PACKAGES/rabbitmq-server-bundle-4.99.99-xyz.1234abcd/)
packaging/

@kartg
Copy link
Contributor Author

kartg commented Mar 7, 2025

I ran the commands that caused the prior PR to fail - #13385 (comment) - and verified that these pass now on the feature branch:

% make clean distclean
 GEN    coverdata-clean
 GEN    clean-dist
 GEN    clean-rabbitmq-server
 GEN    clean-rabbitmq-server-bundle
 GEN    distclean-tmp
 GEN    distclean-kerl
 GEN    distclean-beam-cache
 GEN    distclean-ct
 GEN    distclean-plt
 GEN    distclean-edoc
 GEN    distclean-escript
 GEN    cover-report-clean
 GEN    distclean-packages

% make source-dist RABBITMQ_VERSION=4.1.0-tanzu.392f660ed PRODUCT_VERSION=4.1.0-tanzu.392f660ed PROJECT_VERSION=4.1.0-tanzu.392f660ed
GEN    [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.1.0-tanzu.392f660ed
...
All dependencies are up to date
All dependencies are up to date
 GEN     [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.1.0-tanzu.392f660ed.manifest
 GEN     [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.1.0-tanzu.392f660ed.tar.xz

% ls ./PACKAGES/
rabbitmq-server-4.1.0-tanzu.392f660ed/          rabbitmq-server-4.1.0-tanzu.392f660ed.manifest  rabbitmq-server-4.1.0-tanzu.392f660ed.tar.xz

% make package-generic-unix TARBALL_SUFFIX=generic-unix SOURCE_DIST_FILE=./PACKAGES/rabbitmq-server-4.1.0-tanzu.392f660ed.tar.xz PROJECT_VERSION=4.1.0-beta.392f660ed
 GEN    [redacted]/rabbitmq-server/PACKAGES/rabbitmq-server-4.1.0-beta.392f660ed
...
rm -rf rabbitmq-server-* rabbitmq_server-*
rm -f rabbitmq-server-generic-unix-*.tar.xz

%  ls ./PACKAGES/
rabbitmq-server-4.1.0-beta.392f660ed/                     rabbitmq-server-4.1.0-tanzu.392f660ed/                    rabbitmq-server-generic-unix-4.1.0-beta.392f660ed.tar.xz
rabbitmq-server-4.1.0-beta.392f660ed.manifest             rabbitmq-server-4.1.0-tanzu.392f660ed.manifest
rabbitmq-server-4.1.0-beta.392f660ed.tar.xz               rabbitmq-server-4.1.0-tanzu.392f660ed.tar.xz

I also ran these commands with gmake and they behaved identically with no failures.

@michaelklishin
Copy link
Collaborator

@kartg thank you for sharing the steps you have taken to test these changes.

Copy link
Collaborator

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

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

A generic binary build produced from a source bundle passes a set of client test suites I use (plus rabbitmqadmin v2 tests) and seems as complete and functional as a source-dist-based binary build.

Now lets see how the alpha package workflow runs.

@michaelklishin michaelklishin merged commit 916b032 into rabbitmq:main Mar 8, 2025
268 checks passed
@michaelklishin
Copy link
Collaborator

All packages were built successfully so we can backport.

@michaelklishin
Copy link
Collaborator

@Mergifyio backport v4.1.x v4.0.x

@mergify
Copy link

mergify bot commented Mar 8, 2025

backport v4.1.x v4.0.x

✅ Backports have been created

michaelklishin added a commit that referenced this pull request Mar 8, 2025
(Updated) Adding a "source-bundle" target to the Makefile (backport #13471)
michaelklishin added a commit that referenced this pull request Mar 8, 2025
(Updated) Adding a "source-bundle" target to the Makefile (backport #13471)
@kartg
Copy link
Contributor Author

kartg commented Mar 8, 2025

@michaelklishin thanks for merging this in! Also glad to see that I didn't break any of the packaging workflows this time 😅

Could this change be backported to 3.13.x too ?

michaelklishin added a commit that referenced this pull request Mar 8, 2025
(Updated) Adding a "source-bundle" target to the Makefile

(cherry picked from commit 916b032)

Conflicts:
	Makefile
@michaelklishin
Copy link
Collaborator

@kartg since this is a non-functional change, I've backported manually to v3.13.x. Note that there is a one rsync flag difference, which I doubt would matter but technically forced me to resolve a top-level Makefile conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants