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.

Sign up for GitHub

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

Jump to bottom

makefile support for arm64 packaging #481

Merged
merged 3 commits into from
Jun 28, 2024
Merged

makefile support for arm64 packaging #481

merged 3 commits into from
Jun 28, 2024

Conversation

Zambiorix
Copy link
Contributor

existing targets work as usual

New targets:

package_rpm
package_deb

package_amd64
package_rpm_amd64
package_deb_amd64

package_arm64
package_rpm_arm64
package_deb_arm64

package_base_name

@mperham
Copy link
Collaborator

mperham commented Jun 27, 2024

I don’t think this will build arm64 for release if your goal is released packages.

@Zambiorix
Copy link
Contributor Author

make package does not build release packages?

@mperham
Copy link
Collaborator

mperham commented Jun 27, 2024

It does but where is each arch explicitly built? I don't see where you actually build both arches.

@Zambiorix
Copy link
Contributor Author

make package now builds packages for both amd64 and arm64
I have split the clean target, to perform some cleaning between architecture builds and making sure the output folder is not deleted.

mperham
mperham reviewed Jun 28, 2024
View reviewed changes
Makefile Outdated
upx -qq ./faktory
upx -qq ./$(NAME)

xbuild_arm64: clean generate
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should not be a separate target. xbuild should build both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but they have the same name...
I can put them in a separte folder, perhaps a subfolder in the output folder?
Is cleaning between arch builds required?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Arch subfolders seems fine.

Makefile Outdated
@@ -133,14 +140,20 @@ fmt: ## Format the code
work: ## Run a simple Ruby worker, see also "make run"
cd test/ruby && bundle exec faktory-worker -v -r ./app.rb -q critical -q default -q bulk

clean: ## Clean the project, set it up for a new build
clean_project: ## Clean the project, set it up for a new build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs a better name, clean and clean_project is just confusing.

Makefile Outdated
@@ -183,6 +200,22 @@ rpm: xbuild
faktory=/usr/bin/faktory \
packaging/root/=/

rpm_arm64: xbuild_arm64
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be part of rpm, not a separate target.

Makefile Outdated
@@ -199,6 +232,22 @@ deb: xbuild
faktory=/usr/bin/faktory \
packaging/root/=/

deb_arm64: xbuild_arm64
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

@Zambiorix
Copy link
Contributor Author

Zambiorix commented Jun 28, 2024 •
edited
Loading

Ok, xbuild now builds both archs as faktory_amd64 and faktory_arm64
deb and rpm force copy the output to faktory to keep the package names intact.
They both first start with arm64 and then amd64, leaving the last one undeleted, mimicking the same behavior of this makefile before the change.

clean is updated

.gitignore is updated

@mperham mperham merged commit 7e87525 into contribsys:main Jun 28, 2024
@mperham
Copy link
Collaborator

mperham commented Jun 28, 2024

Thank you, nice work!

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

@mperham mperham mperham left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Zambiorix @mperham