Skip to content

Conversation

@RoiKlevansky
Copy link
Collaborator

No description provided.

@RoiKlevansky
Copy link
Collaborator Author

Hello Guysh. Here is my humble addition to you project.

@RoiKlevansky
Copy link
Collaborator Author

Notice that it currently only builds the native version for x86

@guyush1
Copy link
Owner

guyush1 commented Oct 2, 2024

Hello @RoiKlevansky :)

Thank you for your contribution, much appreciated! Later on we might be able to make it so we regularly release new gdb updates automatically.

@guyush1 guyush1 added the enhancement New feature or request label Oct 2, 2024
@RoiKlevansky RoiKlevansky force-pushed the build-using-dockerfile branch from 4ee8258 to 1d31959 Compare October 3, 2024 16:21
@RoiKlevansky
Copy link
Collaborator Author

I made some great changes to this PR. Now there is a build.sh file, which is even generic and works out of docker. I hope you will find this to your liking.

@RoiKlevansky RoiKlevansky changed the title Build using dockerfile Add automated building using docker Oct 3, 2024
@RoiKlevansky RoiKlevansky changed the title Add automated building using docker Automate building using docker Oct 3, 2024
@guyush1
Copy link
Owner

guyush1 commented Oct 3, 2024

Thank you very much!

I will go over this pr as soon as i am able to

@guyush1 guyush1 marked this pull request as draft October 11, 2024 15:51
@RoiKlevansky RoiKlevansky force-pushed the build-using-dockerfile branch from 1d31959 to 2d46cc4 Compare October 12, 2024 14:31
@RoiKlevansky
Copy link
Collaborator Author

RoiKlevansky commented Oct 12, 2024

Well, it took me some time. But I rewrote the whole thing.

Now, the build system is capable of downloading the packages only once, and keep them for later use. This cuts down on downloads, while also cutting down on disk space (all build targets share the downloads).

@guyush1
Copy link
Owner

guyush1 commented Oct 12, 2024

Well, it took me some time. But I rewrote the whole thing.

Now, the build system is capable of downloading the packages only once, and keep them for later use. This cuts down on downloads, while also cutting down on disk space (all build targets share the downloads).

Your work is much appreciated. I will go over the changes sooni-ish :)

@RoiKlevansky
Copy link
Collaborator Author

Tiny change, I forgot to strip the binaries after build.

@guyush1
Copy link
Owner

guyush1 commented Oct 12, 2024

@RoiKlevansky
Overall this looks really really good! thank you so much for your time & dedication, together we may make this world a little better for the rest of the developer communities :)

I left some comments out in the pr. While it might seem like a lot of comments, most of them are small, and you shall decide what to do with them.

gl hf :)

EDIT: btw, after we finish this issue we publish a new release, since gdb-15.2 released a few days ago.

src/build.sh Outdated
}

if [[ $# -ne 3 ]]; then
>&2 echo "Usage: $0 <target_arch> <build_dir> <gdb_patch>"
Copy link
Owner

Choose a reason for hiding this comment

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

It is a bit weird that you still always re-apply the gdb patches.
maybe you should do it in the dependency downloader script (make it verbose if you decide to do it, so that the user knows he is getting a patches version!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that I have found an even better solution!
I've split the patching mechanism into a separate "patch_gdb.sh" script, which the Makefile runs. A stamp is used in order to guarantee that it is only executed once.

This is quite a bit more elegant (at least I think), and I sure do hope that you'd like it too.

@RoiKlevansky
Copy link
Collaborator Author

RoiKlevansky commented Oct 12, 2024

Thank you for the time you put into this amazing project. You know, it's with people like you that we went to the moon!

Anyway, I'll go over your issues as soon as I can. I want you to know that I already managed to write a GitHub Workflow pipeline for the project. It's even capable of automatically publishing releases!

Here is a demo pipeline.
And here is the demo release.

@RoiKlevansky RoiKlevansky force-pushed the build-using-dockerfile branch from f837607 to 6d07d3a Compare October 13, 2024 22:18
@RoiKlevansky
Copy link
Collaborator Author

I think that I'm kind of reaching the state where I'm ok with the design choices of this PR. It feels like no hacks are needed!

@RoiKlevansky
Copy link
Collaborator Author

I think that right now, the only thing about this PR that sends shivers down my spine is the Makefile. Still, I don't see any way of making it any simpler.

At the end of the day, most Makefiles are weird, it's nothing new.

@guyush1 guyush1 marked this pull request as ready for review October 16, 2024 20:18
@RoiKlevansky
Copy link
Collaborator Author

Glad to see you're back!

@guyush1
Copy link
Owner

guyush1 commented Oct 17, 2024

Glad to see you're back!

I am actually on vacation right now :)
I finished reviewing your pr. resolve / reply to all of the comments and we can merge this :)

@guyush1
Copy link
Owner

guyush1 commented Oct 25, 2024

@RoiKlevansky any updates on this m8 :) ?

@RoiKlevansky RoiKlevansky force-pushed the build-using-dockerfile branch from 6d07d3a to 4cde133 Compare October 29, 2024 18:59
@RoiKlevansky
Copy link
Collaborator Author

I fixed some of the issues and left comments on the ones I didn't fix.

I sure do hope that it will get merged soon!

Copy link
Owner

@guyush1 guyush1 left a comment

Choose a reason for hiding this comment

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

Thank you so much for your effort & time! much appreciated!

@guyush1 guyush1 merged commit fdc2d2e into guyush1:develop Oct 29, 2024
@RoiKlevansky
Copy link
Collaborator Author

It's been a pleasure! Now, let's continue conquering our goals!

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants