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

feat(release): Also bundle dragonfly-debug builds #2424

Merged
merged 15 commits into from
Feb 12, 2024
Merged

feat(release): Also bundle dragonfly-debug builds #2424

merged 15 commits into from
Feb 12, 2024

Conversation

Pothulapati
Copy link
Contributor

No description provided.

@Pothulapati
Copy link
Contributor Author

This mostly works but somehow the regression tests keep failing: https://github.com/dragonflydb/dragonfly/actions/runs/7593673964/job/20684393614

@romange Any idea?

@Pothulapati
Copy link
Contributor Author

We seem to get the debug binaries with this change: https://github.com/dragonflydb/dragonfly/actions/runs/7863662234

(Now, Remving all the changes for testing)

Pothulapati
Pothulapati commented Feb 12, 2024
View reviewed changes
Makefile Outdated
@@ -38,6 +38,7 @@ build:

package:
cd $(RELEASE_DIR); \
tar cvfz $(RELEASE_NAME)-debug.tar.gz dragonfly ../LICENSE.md; \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual change. Essentially we are storing a -debug.tar.gz wih the dragonfly binary before its stripped of symbols and then expect the release pipeline to also publish these files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why there is a whitespace change? Can you please remove unnecessary changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Linter did its thing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, just to make sure the naming is not misleading, I suggest that we call it $(RELEASE_NAME)-dbgsym.tgz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@Pothulapati Pothulapati requested a review from romange February 12, 2024 06:56
@romange romange merged commit 21e7257 into main Feb 12, 2024
10 checks passed
@romange romange deleted the tar/debug-release branch February 12, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romange romange romange approved these changes

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
@Pothulapati @romange