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

Build: Debian/Ubuntu: Avoid incorrect -dirty version suffix #2802

Merged
merged 1 commit into from
Aug 25, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions distributions/debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ endif
%:
dh $@

override_dh_update_autotools_config:
# dh_update_autotools_config replaces libs/opus/config.{sub,guess}.
# This is unnecessary as we don't build opus via autotools at all
# (we use qmake). In addition, this would cause our -dev version generation
# logic to mark Debian builds as -dirty by default.
Copy link
Member

Choose a reason for hiding this comment

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

I'd add a link to this PR here

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, why do that here? We don't add Github Issue/PR links for other code parts either. If you think that something is missing in the comment, I'm happy to add it. If someone wants to track that back to Github, this should be possible using git blame and git log.

Copy link
Member

@ann0see ann0see Aug 25, 2022

Choose a reason for hiding this comment

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

You describe an issue which is fixed. "In addition ... by default." Therefore I thought it would make sense to link this PR for further information.

You could also change the grammar: ... this causes --> This would have caused...

Which would – at least for me – make the reference to this PR less "needed" in the code itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've added the would part in the hope that it makes it clearer. The point of the comment is to state that there are two reasons:

  • First, the config.guess replacement behavior is unnecessary, as the file is not used at all.
  • Second (therefore "in addition"), it causes the -dirty behavior.

# Therefore, disable this behavior:
:

override_dh_auto_configure:
mkdir -p build-gui && cd build-gui && $(QMAKE) "CONFIG+=noupcasename" PREFIX=/usr ../Jamulus.pro
mkdir -p build-nox && cd build-nox && $(QMAKE) "CONFIG+=headless serveronly" TARGET=jamulus-headless PREFIX=/usr ../Jamulus.pro
Expand Down