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

Log build date, git branch and commit hash #3970

Closed
low-batt opened this issue Oct 6, 2022 · 7 comments · Fixed by #3971
Closed

Log build date, git branch and commit hash #3970

low-batt opened this issue Oct 6, 2022 · 7 comments · Fixed by #3971

Comments

@low-batt
Copy link
Contributor

low-batt commented Oct 6, 2022

What you want IINA to do:
During startup emit a log message giving information about the build including the date, git branch and commit hash.

What IINA does currently:
From an IINA log file it is not obvious the executable is from a development build.

Why you think this should be added:
For developers that take development builds to other machines for testing it is desirable to have a way to distinguish between development builds.

I will be posting a PR containing the changes needed to emit such a log message.

Examples of other projects that have something similar:
I looked, but did not find a standard convention for addressing this.

If you build FFmpeg this is what you get for a version number:

ffmpeg version N-107541-g81ebf40efa Copyright (c) 2000-2022 the FFmpeg developers
  built with Apple clang version 13.1.6 (clang-1316.0.21.2.5)

The 100221 is the branch's commit count, the 81ebf40efa is a shortened commit hash.

Some Apple developers change the version number to be an ISO formatted build date with all characters stripped out. I thought it was better to avoid using the application version given Apple's restrictions on what the version can contain.

@low-batt low-batt self-assigned this Oct 6, 2022
low-batt added a commit that referenced this issue Oct 7, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
@low-batt
Copy link
Contributor Author

low-batt commented Oct 7, 2022

With the changes in the PR IINA will emit this log message during startup:

00:37:56.068 [iina][d] Built Oct 7, 2022 at 12:37:52 AM from branch fix-3970, commit 0814f70501caac961ac60ab048b5b1ab06252e62

low-batt added a commit that referenced this issue Oct 7, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
low-batt added a commit that referenced this issue Oct 7, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
low-batt added a commit that referenced this issue Oct 7, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
@svobs
Copy link
Contributor

svobs commented Oct 7, 2022

Very much needed! Though it would also be useful to include something in the UI somehow, in particular something with an increasing number, this should help a lot with bug reports.

@low-batt
Copy link
Contributor Author

low-batt commented Oct 7, 2022

Not sure how this would help with bug reports, other than in the case where someone is using their own build. It would identify that case.

You have guessed phase 2. The ability for a developer to quickly find this information in the UI rather than having to dig into the log file.

I kept UI changes separate as that will require more time from reviewers and likely back and forth as there are lots of style choices with the UI. At the moment reviewers need to focus on other stuff. I was planning on bringing up a UI change once a lot of the outstanding PRs have been reviewed and merged. This PR adds the information a UI would need. So a step in the right direction.

I was frustrated trying to find an accepted best practice for how to handle this. Lots of hacky solutions out there. Semantic Versioning has a concept of including build metadata in the version number. But that can't be done with an Apple version number.

Having some sort of an increasing number is always somewhat problematic to add. There is the git commit count:

low-batt@gag iina (develop $=)$ git rev-list --count HEAD
3080

But for the stated use case both the commit hash and count are not that useful as a developer build frequently includes uncommitted changes. So I feel it is best to use the build date as the "increasing number".

The Xcode build phase that runs the script that updates this information has the IINA executable as a dependency. So the build date is updated whenever the IINA executable changes. Changes to iina-cli or the browser plugin won't update the build information. You would have to do a clean build if you wanted it updated.

For the UI I was thinking of exposing the build date in the About window. Initially hidden. Revealed by some sort of user action. Similar to how clicking on the macOS version number in About This Mac window shows build information.

@svobs
Copy link
Contributor

svobs commented Oct 10, 2022

Not sure how this would help with bug reports, other than in the case where someone is using their own build. It would identify that case.

It's harsh to say but I just don't completely trust what the users say when they provide that info themselves. Human memory is an incredibly unreliable storage medium and is best avoided if possible. And let's not get started on complications with human information gathering and motivation... I think the most common error is that they forgot or didn't realize they upgraded, or misplaced the timeline of the upgrade in their head relative to the bug occurrence. Although often enough they know they upgraded but don't feel like retesting. Having the build identifier right there in the log file provides useful validation and takes just enough effort to fake that it's a good check against these cases.

But for the stated use case both the commit hash and count are not that useful as a developer build frequently includes uncommitted changes. So I feel it is best to use the build date as the "increasing number".

Well...a version which includes uncommitted changes isn't really that useful, is it? Strictly speaking, developers should only be discussing a version of the code which is well-defined to all involved - anything with local changes is a threat to reproducibility. So if nothing else, the Git hash seems to be the most precise from a developer's perspective because it can be looked up and resolved to an exact version, and it's even agnostic about dates and branches. A developer could share a build which came from a commit in their Github repository, for instance, and it would still work.

I think an increasing build number would be more applicable for the end user, because they're only going to be concerned with going in one direction over a line of "official" releases or nightly builds. In that sense, maybe a good solution would be to have one version format if the build came from the official "develop" branch, in which case an increasing build number would be most appropriate; and another, separate version format for non-official versions, where it might be best to omit any implication of linearity.

@low-batt
Copy link
Contributor Author

Yes, if you are passing builds around it should be based on a commit, especially if it is between people. What I was trying to address is the developer that takes builds between their own machines. Like to test on different versions of macOS, or for longer term testing. I do this and sometimes wonder if I copied the version I intended to a machine.

You got me to code up the UI change I was thinking of:
issue-about-window-build-info

The extra build info is initially hidden. It only appears if you option-click in between the mpv version and the license button.

If this proposal to add the info to the log is merged I will propose the UI change.

We do have a problem with users not supplying info. There is more information I would like to add to the log file. Unfortunately some of the basic information is not straight forward to obtain under macOS.

@svobs
Copy link
Contributor

svobs commented Oct 11, 2022

Nice! :) Great to see the Git branch name could be listed. Am I right in guessing that the GitHub URL will change as well?

Yeah, I myself tend to save various packaged versions, so I've fallen into the trap of running different builds concurrently on the same machine. Fortunately it doesn't seem to hurt things too much, other than messing up test results.

We do have a problem with users not supplying info. There is more information I would like to add to the log file. Unfortunately some of the basic information is not straight forward to obtain under macOS.

Seems like it would be nice to grab the version of MacOS and basic hardware info. But I can imagine that might be something Apple would lock down.

@low-batt low-batt linked a pull request Oct 17, 2022 that will close this issue
2 tasks
low-batt added a commit that referenced this issue Nov 9, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
low-batt added a commit that referenced this issue Nov 10, 2022
Updated to address review comments.

This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
uiryuu pushed a commit that referenced this issue Nov 10, 2022
This commit will:
- Add custom keys to Info.plist to contain build information
- Enable preprocessing of the Info.plist file in the Xcode build
- Enable use of an Info.plist header file in the Xcode build
- Add a shell script write_info_plist_header.sh that generates the
  Info.plist header file
- Add a new Xcode build phase to execute the script
- Add a new logBuildDetails method to AppDelegate

With these changes IINA will log a message during startup containing the
build date, git branch and commit hash.
@uiryuu uiryuu reopened this Nov 10, 2022
@low-batt
Copy link
Contributor Author

IINA 1.3.2 contains this enhancement.

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

Successfully merging a pull request may close this issue.

3 participants