[Reverted] Adding a "source-bundle" target to the Makefile

[kartg]

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.

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

[kartg]

I have added tests that prove my fix is effective or that my feature works
All tests pass locally with my changes

I have not added any tests for the Makefile target but i've confirmed the following via local testing:

• The output of source-dist remains unchanged (ignoring build info)

% diff <(sed 's#\+beta.*/##g' main.manifest) <(sed 's#\+beta.*/##g' source-dist.manifest)

1c1
< rabbitmq-server-4.1.0+beta.4.58.gcc0458a
---
> rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5

% diff <(tar -tf main.tar.xz | sed 's#\+beta.*/##g') <(tar -tf source-dist.tar.xz | sed 's#\+beta.*/##g')
%

• The only difference between source-dist and build-dist is with the inclusion of the packaging and test directories, and the additional --include files

% comm -13 source-dist.manifest build-dist.manifest | wc -l
    1363

% comm -13 source-dist.manifest build-dist.manifest | grep -c "test"
1327

% comm -13 source-dist.manifest build-dist.manifest | grep -c "packaging"
16

% comm -13 source-dist.manifest build-dist.manifest | grep -v "test" | grep -v "packaging"
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/autoscaling-group
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/autoscaling-group/main.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/autoscaling-group/outputs.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/autoscaling-group/setup-vms.sh
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/autoscaling-group/variables.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/main.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/outputs.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/setup-vms.sh
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/templates
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/templates/setup-erlang.sh
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/direct-vms/variables.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/vms-query
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/vms-query/main.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/vms-query/outputs.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/vms-query/query-vms.sh
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/terraform/vms-query/variables.tf
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/tls-certs
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/tls-certs/Makefile
rabbitmq-server-4.1.0+beta.4.59.gaa9e0a5/deps/rabbitmq_ct_helpers/tools/tls-certs/openssl.cnf.in

% comm -13 <(tar -tf source-dist.tar.xz) <(tar -tf build-dist.tar.xz) | wc -l
    1363
% comm -13 <(tar -tf source-dist.tar.xz) <(tar -tf build-dist.tar.xz) | grep -c "test"
1327
% comm -13 <(tar -tf source-dist.tar.xz) <(tar -tf build-dist.tar.xz) | grep -c "packaging"
16
% comm -13 <(tar -tf source-dist.tar.xz) <(tar -tf build-dist.tar.xz) | grep -c "tools"
22

[michaelklishin]

@kartg thank you, we will discuss this with the team. We don't have any tests for the Make targets, usually any serious issue would break packaging/alpha builds.

I'm afraid the word "build" may be misleading here. It suggests that it might have something to do with producing binary builds, a task handled by rabbitmq/rabbitmq-packaging and server-packages (or their alternatives).

[kartg]

I'm afraid the word "build" may be misleading here. It suggests that it might have something to do with producing binary builds, a task handled by rabbitmq/rabbitmq-packaging (or its alternative).

@michaelklishin Fair point 😄 Any suggestions on a more accurate term? Maybe "bundle" or "snapshot" ?

[michaelklishin]

@kartg if the focus of these targets is on validation, how about "validated source bundle" or just "source bundle"?

[kartg]

source-bundle sounds good to me 👍 I'll make those updates.

[kartg]

thank you! 😀

[michaelklishin]

@kartg since this is a non-functional change, I'll backport it to v3.13.x.

[michaelklishin]

since this is a non-functional change, I'll backport it to v3.13.x.

Done.

[michaelklishin]

@kartg this breaks

gmake package-generic-unix

in this repo, so I will revert. I cannot immediately tell why but it can be reproduced in multiple different environments.

https://github.com/rabbitmq/server-packages/actions/runs/13468260488/job/37638171804 is currently waiting for a runner but it should fail there.

[michaelklishin]

@kartg to reproduce the failure locally

gmake source-dist RABBITMQ_VERSION=4.1.0-tanzu.392f660ed PRODUCT_VERSION=4.1.0-tanzu.392f660ed PROJECT_VERSION=4.1.0-tanzu.392f660ed

ls -l ./PACKAGES/

gmake 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

[kartg]

@michaelklishin thanks for those commands. I was able to reproduce the failure and track down the root cause. I'm looking for a recommendation in choosing a solution, because there are tradeoffs with either approach.

I'd also like to bring to your attention two potential bugs, and see if it's something we want to fix.

---

This failure was due to a bug in my change, which didn't account for the fact that the rsync code path could be triggered by make targets other than source-dist and source-bundle. AFAIK, the "right" way to fix this is to separate the dependency paths for those two targets because they are looking to do the same thing, just with different contents. This way, even if they are invoked together (make source-dist source-bundle), we get the desired output (two distinct tarballs with different contents). However, I feel like this approach would result in a lot of duplicated code in the Makefile.

An alternative way to resolve this issue is to have the pre-requisite target check for "source-bundle" in the make command's input and switch the RSYNC_FLAG variable based on this:

$(SOURCE_DIST): RSYNC_FLAGS = $(if $(filter source-bundle,$(MAKECMDGOALS)),$(SOURCE_BUNDLE_RSYNC_FLAGS),$(SOURCE_DIST_RSYNC_FLAGS))                                                                                               
$(SOURCE_DIST): $(ERLANG_MK_RECURSIVE_DEPS_LIST)
....

This avoids duplicate code and is a far smaller change. However this does mean that using "source-bundle" and "source-dist" together will result in an incorrect output for one of those targets (and only a single tarball).

How would you recommend that I proceed?

---

As for the potential bugs:

1. The gmake package-generic-unix you posted above uses a different PROJECT_VERSION value than the one in the preceding gmake source-dist command. Is this intentional?
2. When invoking gmake package-generic-unix any user-supplied values of SOURCE_DIST_FILE are ignored because the top-level Makefile overrides this when invoking the target under packaging/

Please let me know if I should open issues for these.

[michaelklishin]

@kartg I don't have a strong opinion on how the make targets are organized, extracting common code makes sense to me.

As for PROJECT_VERSION vs VERSION, both are used, plus RPM and Windows installer packages have their own versions (that use different formats, e.g. X.Y.Z~N or A.B.C.D.
PROJECT_VERSION is the most recent addition and if it is not set, the version will be automatically computed by erlang.mk, which means it will be incorrect. That's why the 4.0.0 and 4.0.1 version had incorrect reported versions in the generic UNIX package.

I haven't invoked the package-generic-unix target from under ./packaging and I don't think anyone does that. Our Actions workflows do not and neither do RabbitMQ core team members as far as I can tell.

[michaelklishin]

Re-submitted in #13471.