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.

Sign up for GitHub

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

Jump to bottom

qesteidutil: Fixes build from Qt upgrade. #46083

Merged
merged 1 commit into from Sep 23, 2018
Merged

qesteidutil: Fixes build from Qt upgrade. #46083

merged 1 commit into from Sep 23, 2018

Conversation

samueldr
Copy link
Member

@samueldr samueldr commented Sep 5, 2018 •
edited

Motivation for this change

Plowing through #45960 failures.

This is (with at least another failure) fallout from Qt 5.10 -> 5.11 from June.

The patch does not strictly address the issue, but has been judged (by me) to be fine; the actual intended fix is minimal, and the value of having one fewer failure is good. If it is judged not to be fine, cutting the fix and keeping the header changes in a custom patch can be done.

It'll need cherry-picking to release-18.09

Things done
  • ✔️ Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • ✔️ NixOS
    • ⬜ macOS
    • ⬜ other Linux distributions
  • ⬜ Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • ⬜ Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • 🌶️ Tested execution of all binary files (usually in ./result/bin/)
  • ⬜ Determined the impact on package closure size (by running nix path-info -S before and after)
  • ✔️ Fits CONTRIBUTING.md.

#44047 stops me from testing this :/.

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: qesteidutil

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: qesteidutil

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10
shrinking /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10/bin/qesteidutil
gzipping man pages under /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10/share/man/
strip is /nix/store/y4ymnvgxygpq05h03kyzbj572zmh6zla-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10/bin
patching script interpreter paths in /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10
checking for references to /build in /nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10...
postPatchMkspecs
/nix/store/92i0yk7qh6vza1a3sbgmff6bcr2h2bil-qesteidutil-3.12.10

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: qesteidutil

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10
shrinking /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10/bin/qesteidutil
gzipping man pages under /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10/share/man/
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10/bin
patching script interpreter paths in /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10
checking for references to /build in /nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10...
postPatchMkspecs
/nix/store/616675cm14km8gkiv5dkjyhz69cs3lbf-qesteidutil-3.12.10

@xeji
Copy link
Contributor

xeji commented Sep 5, 2018

... cutting the fix and keeping the header changes in a custom patch can be done.

Since this is a security-sensitive application, we should only apply appstream patches, not try to make our own, so I think it's fine.

@xeji
Copy link
Contributor

xeji commented Sep 5, 2018

Now I may be overly cautious here, but after applying the patch it's not the officially released version any more, so we should even think about marking it as insecure and warning users about that.

After all it's a government-sponsored app related to electronic id cards, and users deserve to know they're not installing the official release (which unfortunately doesn't build).

@samueldr
Copy link
Member Author

samueldr commented Sep 8, 2018

I was about to open an issue on their repo, but first I was prompted to read the guidelines.

Part of it is as such:

Official builds are provided through the official distribution point https://installer.id.ee. If you want support, you have to be using official builds.

Source code is provided on "as is" terms with no warranty (see LICENSE for more information). Do not file Github issues with generic support requests.

So yeah, it looks like this already isn't official. Though I additionally agree that we shouldn't willy-nilly patch this!

@jagajaga you are marked as the maintainer. Any input? Do you have any association with this software? Are you a user?

@xeji
Copy link
Contributor

xeji commented Sep 8, 2018

Instead of patching, we could use the latest git revision, name the nixpkgs version unstable-2018-xx-xx and add something like "unsupported development version, use installer... for the official version" to the description.

@jagajaga
Copy link
Member

jagajaga commented Sep 8, 2018

We are fetching it from "official" repository, so everything is alright. I'm using this software everyday on nixos. Let's merge this.

@xeji
Copy link
Contributor

xeji commented Sep 23, 2018

Sorry this got delayed. Since the maintainer approves, let's merge it.

@xeji xeji merged commit ef2b217 into NixOS:master Sep 23, 2018
xeji pushed a commit that referenced this pull request Sep 23, 2018
@samueldr @xeji
qesteidutil: Fixes build from Qt upgrade. (#46083)
7c7f962 
(cherry picked from commit ef2b217)
@xeji
Copy link
Contributor

xeji commented Sep 23, 2018

backported: 7c7f962

@samueldr samueldr deleted the zhf/qesteidutil branch February 12, 2019 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@samueldr @GrahamcOfBorg @xeji @jagajaga