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

Identify BiT dev build by git hash #1637

Closed
ptilopteri opened this issue Feb 4, 2024 · 21 comments · Fixed by #1640 or #1641
Closed

Identify BiT dev build by git hash #1637

ptilopteri opened this issue Feb 4, 2024 · 21 comments · Fixed by #1640 or #1641
Assignees
Labels
Discussion decision or consensus needed Feature requests a new feature Feedback needs user response, may be closed after timeout without a response

Comments

@ptilopteri
Copy link

BiT development builds report version as "backintime 1.4.4-dev" which actually provides little identification.

I propose appending the last commit id to "backintime 1.4.4-dev.c65fc91" so one can tell that his dev version needs updating

@buhtz
Copy link
Member

buhtz commented Feb 5, 2024

Dear ptilopteri,
thanks for your report.

I like that idea. And we have this still implemented somehow. If BIT starts from a git repo then you will find the current git hash in the About dialog.

backintime/qt/app.py

Lines 1842 to 1846 in c65fc91

ref, hashid = tools.gitRevisionAndHash()
git_version = ''
if ref:
git_version = " git branch '{}' hash '{}'".format(ref, hashid)
name = QLabel('<h1>' + self.config.APP_NAME + ' ' + version + '</h1>' + git_version)

But I also support the idea to add the git hash (and branch name maybe) to the version string and also maybe in the window title.

Next time when reporting please consider the use of correct upper case letters (e.g. beginn of sentences, the word "I", ...). It is then more easier to read for us and others. Thanks in advance.

@buhtz buhtz self-assigned this Feb 5, 2024
@buhtz buhtz added Feature requests a new feature Discussion decision or consensus needed GOOD FIRST ISSUE Used by 24pullrequests.com to suggest issues labels Feb 5, 2024
@buhtz buhtz added this to the Upcoming release (1.5.0) milestone Feb 5, 2024
@buhtz buhtz changed the title identify BiT dev build Identify BiT dev build by git hash Feb 5, 2024
@ptilopteri
Copy link
Author

ptilopteri commented Feb 5, 2024 via email

@buhtz
Copy link
Member

buhtz commented Feb 5, 2024

Yes I agree that the git hash could be an element of the regular version string (only in dev versions of course).
But I don't decide this own my own. Let's wait for arguments of other team members.

I forgot to mention that also --diagnostics show the git infos.

$ ./backintime --diagnostics
{
    "backintime": {
        "git-project-root": "/home/UsernameReplaced/mluCloud/my.work/bit/backintime",
        "git-branch": "fix/1634restrictpylint",
        "git-hash": "8c3f7ff6ba063115f56df8d40913afdabc175fa3"
    }
}

@emtiu
Copy link
Member

emtiu commented Feb 5, 2024

If we make sure to disable this for regular releases, I see no disadvantages :)

@buhtz
Copy link
Member

buhtz commented Feb 5, 2024

If we make sure to disable this for regular releases, I see no disadvantages :)

The current code works only if it runs from a git repo.

After doing ./configure && make && sudo make install the code won't give you any git information because BIT is then installed into the OS and not aware of a git repo anymore.

I am not a pro when it comes to make. Maybe we could modify the configure script somehow that it takes the current git-hash (if there is one) and store it in a separate file (e.g. /usr/share/backintime/build_version). If BIT finds this file it will present this information. Just a quick n dirty idea. I am not sure about it.

FYI: The version string of BIT is "hardcoded" into the VERSION file. We can not manually add a git hash every time we do a commit.

@ptilopteri
Copy link
Author

ptilopteri commented Feb 5, 2024 via email

@buhtz
Copy link
Member

buhtz commented Feb 5, 2024

not for what I just built

You build/installed BIT from the cloned repo via ./configure && make && sudo make install. I can see this from your output: "started-from": "/usr/share/backintime/common",. That is technically the reason why you can not see the git hash anymore. Technically you need to run BIT from the git repo without installing it.

I agree this is a shortcoming and we should find a solution for this.

And I need to correct my last post: The version is not "hardcoded" in VERSION file on runtime level. We have a helper script update_version.sh which take the version number from that file and then inject it everywhere in the py files (and some other files) where it is relevant.

To my knowledge of the Filesystem Hierarchy Standard (FHS) a file containting the current git hash (and other static meta information) should be generated/installed into /usr/share/backintime/common and /usr/share/backintime/qt folder. Two folders because technically we do have two separate applications.

EDIT:
Note to myself: There is a redundancy in code. The --diagnostics feature get git infos from diagnostics.py::get_git_repository_info() but the about dialog from tools.py::gitRevisionAndHash().

EDIT2: Adding this line in the Makefile to get branch and hash in json format.

python3 -c "import diagnostics as d;import pathlib as p;import json as j;git=d.get_git_repository_info(p.Path.cwd().parent);print(j.dumps(git))" > bit.git.json

Of course replace bit.git.json with the correct path (e.g. /usr/share/backintime/common/bit.git.json and the same for qt).

$ cat bit.git.json
{"branch": "fix/1634restrictpylint", "hash": "8c3f7ff6ba063115f56df8d40913afdabc175fa3"}

@buhtz
Copy link
Member

buhtz commented Feb 6, 2024

I have a solution in mind and I would like to present the details in a PR. Before that I would like to know if there are arguments against having such an extra file in /usr/share/backintime/common and /usr/share/backintime/qt containing the branch at git hash at build time?

@emtiu
Copy link
Member

emtiu commented Feb 6, 2024

I have a solution in mind and I would like to present the details in a PR. Before that I would like to know if there are arguments against having such an extra file in /usr/share/backintime/common and /usr/share/backintime/qt containing the branch at git hash at build time?

Hmm. I'm not well-informed on this topic, but it seems a bit like overkill.

How do other projects do it? There's probably a lot of software that includes its git version in shipped packages.

Is this something that might work more "naturally" once we eliminate make and move to "modern" python packaging?

@buhtz
Copy link
Member

buhtz commented Feb 6, 2024

it seems a bit like overkill.

Yes, I also would treat this as an temporary workaround until we find a better solution.

How do other projects do it? There's probably a lot of software that includes its git version in shipped packages.

Good question. I guess that C/C++ and other compile-linking languages do inject the version string into their source files (e.g. via configure/make) before starting the compiler. But it is just a guess.

Is this something that might work more "naturally" once we eliminate make and move to "modern" python packaging?

Very good question! 😄 I will ask the community. But to my current low expertise on that topic I would add a "custom build step" to inject git info somewhere in the source files. I am not sure. It feels also like a workaround or hack.

@buhtz
Copy link
Member

buhtz commented Feb 6, 2024

About a pythonic solution: It seems to be that there are plugins for the several Python build-tools (e.g. setuptools, hatchling) solving that problem without hacking around. I will investigate a solution further in my demo project.

Anyway. This won't happen early. As a short solution and workaround I currently see only this extra file generated while make install.

Maybe Jürgen will have another idea.

@ptilopteri
Copy link
Author

ptilopteri commented Feb 6, 2024 via email

@ptilopteri
Copy link
Author

ptilopteri commented Feb 6, 2024 via email

@aryoda
Copy link
Contributor

aryoda commented Feb 8, 2024

The point is that the git commit hash shall only be injected into a non-release build.

So we need either

  • an automatic way to recognize 100 % correctly if make ... is building a release or dev version
  • or add a different make target to build releases (eg. make release) to let the (human) builder decide if the commit hash shall be part of the version number

I propose to extend our configure script

  • to append the (or replace an existing) commit hash to the version.txt only if it contains the word dev
  • and then always call the updateversion.sh

PS: The commit hash is a hex number so it may never contain the word dev

@buhtz
Copy link
Member

buhtz commented Feb 8, 2024

The point is that the git commit hash shall only be injected into a non-release build.

I would suggest a slightly different and IMHO easier to implement approach. The version.txt (better gitversion.txt?) is created every time. But BIT will read from it only if it finds a dev at the end of its regular version string.

When BIT asking for its version string (via --version, --diagnostics, About dialog or main window title bar) I would suggest this decision cascade:

  1. Running in a git repo? I often start BIT from the repo without installing it. This question is still implemented in the --diagnostics section. TRUE: Read branch and hash from the existing repo. FALSE: Next step.
  2. -dev at the end of the regular version string? TRUE: Read the gitversion.txt file and add the hash to the version string. I would also somehow add the branch name to it or maybe only add the branch name when it is different from "main" or "dev".

This would cover all known to me use cases.

@buhtz buhtz removed the GOOD FIRST ISSUE Used by 24pullrequests.com to suggest issues label Feb 9, 2024
buhtz added a commit that referenced this issue Feb 13, 2024
BIT use the last commit hash in its version string if it is a "*-dev" version.

Additionally some refactoring:
 - "Config.VERSION" moved into new file "version.py" as "__version__"
 - "diagnostics.py::get_git_repository_info()" moved to "tools.py".
 - Removed "tools.py::gitRevisionAndHash()"

Some more
 - File "VERSION" no longer part of "make install" target

Close #1637
@ptilopteri
Copy link
Author

ptilopteri commented Feb 13, 2024 via email

@buhtz
Copy link
Member

buhtz commented Feb 13, 2024

Have you re installed BIT common and BIT qt as described here?

The git-hash is "injected" only when when doing make install.

The git hash is determined only when calling updateversion.sh and then installing BIT common and BIT qt.

./updateversion.sh
VERSION: 1.4.4-dev.98246546
Update 'common/version.py'
Update 'common/doc-dev/conf.py'
Update 'common/man/C/backintime.1'
Update 'common/man/C/backintime-config.1'
Update 'common/man/C/backintime-askpass.1'
Update 'qt/man/C/backintime-qt.1'
Update 'debian/changelog'

This might not fulfill all your needs but it is how we have discussed it the related PR #1640.
If you have further needs please open a new Issue and try to describe your needs and use case as specific as possible.

@buhtz buhtz reopened this Feb 13, 2024
@ptilopteri
Copy link
Author

ptilopteri commented Feb 13, 2024 via email

@buhtz buhtz added the Feedback needs user response, may be closed after timeout without a response label Feb 13, 2024
@buhtz
Copy link
Member

buhtz commented Feb 13, 2024

@aryoda Is that the reason why you wanted to add the "updateversion.sh" call to the "make install" target? Make sense now to me. 🤣

@aryoda
Copy link
Contributor

aryoda commented Feb 13, 2024

@aryoda Is that the reason why you wanted to add the "updateversion.sh" call to the "make install" target? Make sense now to me. 🤣

@buhtz Yes, every "manual protocol" to follow ("step-by-step" TODO lists) may be forgotten or applied in the wrong way so make install is the best place to free our (and the "installers" mind :-)

@aryoda
Copy link
Contributor

aryoda commented Feb 13, 2024

I vote to close this issue as "fixed" since "building" with make install injects the Git hash.

Edit: Looks like make install does not yet contain the call of updateversion.sh but the build target is suggested instead in PR #1641

Starting BiT directly from the repo without installing it does not inject the Git hash but
a) the user knows the Git hash then
b) and this is not valid end user scenario (only for developers or testers)

buhtz added a commit that referenced this issue Feb 15, 2024
The git-hash is added to every "*.dev" version string while calling "make install".

Close #1637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion decision or consensus needed Feature requests a new feature Feedback needs user response, may be closed after timeout without a response
Projects
None yet
4 participants