search

aarond10 / https_dns_proxy

A lightweight DNS-over-HTTPS proxy.
MIT License
775 stars 114 forks source link

version is empty when invoking https-dns-proxy -V on OpenWrt #149

Closed stangri closed 1 year ago

stangri commented 1 year ago

Hello,

Is the version information present on a normal Linux distro? On OpenWrt, I get:

SG-135 in ~ # https-dns-proxy -V

SG-135 in ~ #

Do I need to pass something as a CMAKE_OPTIONS in the Makefile to have it print a valid version information?

Thanks!

aarond10 commented 1 year ago

How is the binary being built? The string returned by -V should be the GIT_VERSION which is set via CMake to either the git version or "UNKNOWN". (See GIT_VERSION in CMakeLists.txt)

I can't see how CMake could be setting it to an empty string which makes me think it might not be being built with CMake at all?

On Wed, 12 Oct 2022 at 09:17, Stan Grishin @.***> wrote:

Hello,

Is the version information present on a normal Linux distro? On OpenWrt, I get:

SG-135 in ~ # https-dns-proxy -V

SG-135 in ~ #

Do I need to pass something as a CMAKE_OPTIONS in the Makefile https://github.com/openwrt/packages/blob/master/net/https-dns-proxy/Makefile to have it print a valid version information?

Thanks!

— Reply to this email directly, view it on GitHub https://github.com/aarond10/https_dns_proxy/issues/149, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTOXVRYI5H4PZWTUSYUDLWCXROFANCNFSM6AAAAAARCW3DIQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

stangri commented 1 year ago

How is the binary being built? The string returned by -V should be the GIT_VERSION which is set via CMake to either the git version or "UNKNOWN". (See GIT_VERSION in CMakeLists.txt) I can't see how CMake could be setting it to an empty string which makes me think it might not be being built with CMake at all? … On Wed, 12 Oct 2022 at 09:17, Stan Grishin @.> wrote: Hello, Is the version information present on a normal Linux distro? On OpenWrt, I get: SG-135 in ~ # https-dns-proxy -V SG-135 in ~ # Do I need to pass something as a CMAKE_OPTIONS in the Makefile https://github.com/openwrt/packages/blob/master/net/https-dns-proxy/Makefile to have it print a valid version information? Thanks! — Reply to this email directly, view it on GitHub <#149>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTOXVRYI5H4PZWTUSYUDLWCXROFANCNFSM6AAAAAARCW3DIQ . You are receiving this because you are subscribed to this thread.Message ID: @.>

I believe on OpenWrt buildbots there's a git installed, however the source is not checked out from git, but rather downloaded as a tar from github, so it may as well output nothing (as far as I understand CMakeLists.txt).

I'm not very familiar with cmake, is it possible to force a specific version thru an environment variable/CMake flag somehow?

stangri commented 1 year ago

Hey @aarond10 , were you able to look into it? I believe the only way to get OpenWrt SDK to check the code directly from github is to intentionally put the wrong hash for the archive in the Makefile. Is there any other way to add a version number and/or can you or @baranyaib90 make changes to the CMakeLists.txt so that the version is derived from a variable/flag I can pass to CMake?

baranyaib90 commented 1 year ago

Hi, stuff is understandable from OpenWRT PoV. But from this project PoV I wanted to have automated version generation instead of some "maintained by hand" version file. I need to think about this. Please don't expect fast solution. I will not support works arounds like the env. var. (If we need to have version file added in the repo, I am thinking about some git hook or github action to update it.)

aarond10 commented 1 year ago

Yeah, there doesn't seem to be a quick fix for this. It's being built in a way that I never really considered well...

I am having flashbacks to CVS but a hook that generates a version seems probably the most universal. It binds the project to git (hub) but that doesn't really bother me. I haven't done anything like that in years. Will try to find an hour or so to play around with it.

On Mon, 24 Oct 2022, 7:15 am baranyaib90, @.***> wrote:

Hi, stuff https://openwrt.org/docs/guide-developer/packages#use_source_repository is understandable from OpenWRT PoV. But from this project PoV I wanted to have automated version generation instead of some "maintained by hand" version file. I need to think about this. Please don't expect fast solution. I will not support works arounds like the env. var. (If we need to have version file added in the repo, I am thinking about some git hook or github action to update it.)

— Reply to this email directly, view it on GitHub https://github.com/aarond10/https_dns_proxy/issues/149#issuecomment-1288191744, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABTOXVUG6GXIOTVMVMAXJDWEWME7ANCNFSM6AAAAAARCW3DIQ . You are receiving this because you were mentioned.Message ID: @.***>

stangri commented 1 year ago

@baranyaib90 @aarond10 thank you for your replies. It's definitely not a big deal (so your time might be better spent elsewhere), I set the version number for the init script in the Makefile, so the built-in CI test runs against that. Would be nice to enable the test for the principal binary as well tho.

baranyaib90 commented 1 year ago

@stangri as a (long term) work around, you could extend your current patch 010-fix-cmakelists.patch with this https://github.com/baranyaib90/https_dns_proxy/commit/2ca80486ba6a4e5acbdf0ff581d9754af17fa33b so you can use CMAKE_OPTIONS += -DCLANG_TIDY_EXE= -DGIT_VERSION=2022.10.15-f52a85

stangri commented 1 year ago

with this baranyaib90@2ca8048

Thank you! Are you going to merge this patch in this repo at some point?

baranyaib90 commented 1 year ago

I don't know yet, if I will propose this in a later pull request or not. At least you have the work-around for OpenWRT build. As I wrote before, I'm against this aproach, but better than nothing. I need to think about this...