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.

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

libdeflate: Update to v1.17 #11788

Merged
merged 3 commits into from Jan 17, 2023
Merged

libdeflate: Update to v1.17 #11788

merged 3 commits into from Jan 17, 2023

Conversation

oliv3r
Copy link
Contributor

@oliv3r oliv3r commented Jan 13, 2023

CMake depends on (libdeflate-)gunzip, libdeflate depends on Cmake, so we can't win.

Luckily libdeflate is very easy to build, without any build system, so lets just manually compile it and be done with it.

Signed-off-by: Olliver Schinagl oliver@schinagl.nl

@github-actions github-actions bot added the build/scripts/tools pull request/issues for build, scripts and tools related changes label Jan 13, 2023
tools/Makefile Outdated Show resolved Hide resolved
tools/libdeflate/Makefile Outdated Show resolved Hide resolved
@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 13, 2023

@neheb btw, how can I force a rebuild of a tool? It keeps telling me 'everything is up to date'. make tools/libdeflate with clean, compile, anything or everything, make oldconfig nothing yields a rebuild ... (i tried searching for libdeflate stamps and artifacts, but found nothing to delete anymore. ...

@robimarko
Copy link
Contributor

You tried changing the PKG_RELEASE?

@neheb
Copy link
Contributor

neheb commented Jan 13, 2023

I think it's because this package is using Host/Clean instead of Host/Uninstall

edit: yeah, need to call Host/Clean/Default in that section.

@Ansuel
Copy link
Member

Ansuel commented Jan 13, 2023

Oh wow it can simply be compiled with good old gcc raw command? Nice!

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 13, 2023

You tried changing the PKG_RELEASE?

Part of the Merge Request already :) but no :( doesn't work.

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 13, 2023

I think it's because this package is using Host/Clean instead of Host/Uninstall

edit: yeah, need to call Host/Clean/Default in that section.

So this goes beyond my knowledge (and desire to know) about the OpenWRT build system :p however, no, doesn't seem to make a difference. I tried several combinations, Host/Uninstall, Host/Clean; adding the call (I did push that bit as I saw other tools having it). But a) I have no idea how all of this magic works, and b) I can't force a rebuild to check if it actually compiles. So feel free to merge it on good faith :) or test it somehow. I can't seem to :(

Btw, I think logically, It would be uninstall, not clean; but what do I now :) (Who actually does know?)

@neheb
Copy link
Contributor

neheb commented Jan 13, 2023

I'll fix it in the other PR.

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 14, 2023

@neheb thank you for fixing it in your MR, i'll close this and refer to yours!

@oliv3r oliv3r closed this Jan 14, 2023
@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 15, 2023

So I figured things here, which due to a version bump makes building even easier.

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 15, 2023

@neheb should be all good now I hope :)

tools/libdeflate/Makefile Outdated Show resolved Hide resolved
@neheb
Copy link
Contributor

neheb commented Jan 15, 2023

Change it to libdeflate: update to 1.17

@oliv3r oliv3r force-pushed the libdeflate branch 2 times, most recently from 8f2a570 to a1f84be Compare January 15, 2023 18:24
@oliv3r oliv3r changed the title libdeflate: Avoid circular dependencies libdeflate: Update to v1.17 Jan 15, 2023
tools/Makefile Outdated Show resolved Hide resolved
@neheb
Copy link
Contributor

neheb commented Jan 15, 2023

Change it to libdeflate: update to 1.17 and keep second commit title lowercase.

@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 16, 2023

Change it to libdeflate: update to 1.17 and keep second commit title lowercase.

"Officially" Linux and in many git guidelines, you are supposed to start your subject with a capital. I'll make it consistent.

@oliv3r oliv3r force-pushed the libdeflate branch 2 times, most recently from d1a5dfd to 24a7bc7 Compare January 16, 2023 18:33
tools/libdeflate/Makefile Outdated Show resolved Hide resolved
@Ansuel
Copy link
Member

Ansuel commented Jan 17, 2023

@oliv3r also can you change (actually fix the PR so that it works) to [ ... ] ?

With the 2 requeste change, I will merge this and then I will merge #11717 rebased

Like with commit ae614fb ("tools: Improve diffability/maintainability")
we also want tools-core to be easy to maintain. While a smaller target,
it's still usefull and makes things nice and consistent.

To avoid duplicating any tools in the comment, simplify the comment
instead.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
The new version of libdeflate makes it a little easier to build it
without any build system.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 17, 2023

@oliv3r also can you change (actually fix the PR so that it works) to [ ... ] ?

With the 2 requeste change, I will merge this and then I will merge #11717 rebased

Done! Lets get this puppy in!

@Ansuel
Copy link
Member

Ansuel commented Jan 17, 2023

@oliv3r missing separator error

@Ansuel
Copy link
Member

Ansuel commented Jan 17, 2023

There are some rebase comment left

tools/Makefile Outdated Show resolved Hide resolved
CMake depends on (libdeflate-)gunzip, libdeflate depends on Cmake, so we
can't win.

Luckily libdeflate is _very_ easy to build, without any build system, so
lets just manually compile it and be done with it.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@Ansuel
Copy link
Member

Ansuel commented Jan 17, 2023

@oliv3r just in case this is rebased on top of master so i can merge without removing your verified tag?

@openwrt-bot openwrt-bot merged commit 62e1509 into openwrt:master Jan 17, 2023
@oliv3r oliv3r deleted the libdeflate branch January 17, 2023 22:31
@oliv3r
Copy link
Contributor Author

oliv3r commented Jan 17, 2023

@oliv3r just in case this is rebased on top of master so i can merge without removing your verified tag?

Not sure what you meant, but it should have been rebased on master (as I always do a fetch; rebase; cycle), but since it's merged, I guess its good :)

@Ansuel
Copy link
Member

Ansuel commented Jan 17, 2023

(to give you some context... to flag pr as merged they needs to be merged with --ff-only so they have to be rebased on top of master... and by rebasing on top of master i would have dropped your verified tag as i would had to resign them for the force push... but they were rebased so np :D)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/scripts/tools pull request/issues for build, scripts and tools related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants