Skip to content

mrsh: 2020-01-08 -> 2020-07-27 etc. #101910

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
Merged
merged 1 commit into from
Nov 2, 2020
Merged

mrsh: 2020-01-08 -> 2020-07-27 etc. #101910

merged 1 commit into from
Nov 2, 2020
+6 −3

Conversation

omasanori
Copy link
Contributor

Motivation for this change

This change updates mrsh to a newer revision. The changes between them are available.

Moreover, it enables doCheck as executing the whole test suites takes just few seconds.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@omasanori
Copy link
Contributor Author

ping @matthiasbeyer (as ofborg did not set a reviewer for some reason.)

matthiasbeyer
matthiasbeyer approved these changes Oct 28, 2020
View reviewed changes
Copy link
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

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

diff LGTM

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Oct 28, 2020 •
edited
Loading

Result of nixpkgs-review pr 101910 1 run on x86_64-darwin

1 package failed to build:
  • mrsh
Library readline found: YES
Checking for function "rl_replace_line" with dependency -lreadline: YES
Checking if "-Wl,--version-script" links: NO
Compiler for C supports link arguments -Wl,-exported_symbols_list /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/libmrsh.darwin.sym: NO

meson.build:71:1: ERROR: Problem encountered: Linker doesn't support --version-script or -exported_symbols_list

A full log can be found at /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/build/meson-logs/meson-log.txt

@omasanori
Copy link
Contributor Author

@SuperSandro2000 Perhaps it assumes that the compiler is Clang on macOS: https://github.com/emersion/mrsh/blob/0da902c0ee6f443fe703498e60f266af7f12537e/meson.build#L67-L70 I have no access to macOS so it is just a guess though.

@omasanori omasanori force-pushed the mrsh/update-2020-07-27 branch from 1f13c0b to 7004b98 Compare October 30, 2020 05:09
@omasanori
Copy link
Contributor Author

@SuperSandro2000 Would you mind if I ask for retest on macOS?

@omasanori omasanori changed the title mrsh: 2020-01-08 -> 2020-07-27, enable check mrsh: 2020-01-08 -> 2020-07-27 etc. Oct 30, 2020
@SuperSandro2000
Copy link
Member

@SuperSandro2000 Perhaps it assumes that the compiler is Clang on macOS: emersion/mrsh@0da902c/meson.build#L67-L70 I have no access to macOS so it is just a guess though.

I am not 100% sure but I think so.

@SuperSandro2000 Would you mind if I ask for retest on macOS?

Not at all. Added to the list.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 101910 run on x86_64-linux 1

1 package built:
  • mrsh

@SuperSandro2000
Copy link
Member

Still failing for darwin:

Host machine cpu family: x86_64
Host machine cpu: x86_64
Compiler for C supports arguments -Wundef: YES
Compiler for C supports arguments -Wlogical-op: NO
Compiler for C supports arguments -Wmissing-include-dirs: YES
Compiler for C supports arguments -Wold-style-definition: YES
Compiler for C supports arguments -Wpointer-arith: YES
Compiler for C supports arguments -Winit-self: YES
Compiler for C supports arguments -Wfloat-equal: YES
Compiler for C supports arguments -Wstrict-prototypes: YES
Compiler for C supports arguments -Wredundant-decls: YES
Compiler for C supports arguments -Wimplicit-fallthrough=2: NO
Compiler for C supports arguments -Wendif-labels: YES
Compiler for C supports arguments -Wstrict-aliasing=2: YES
Compiler for C supports arguments -Woverflow: YES
Compiler for C supports arguments -Wformat=2: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wno-missing-braces: YES
Compiler for C supports arguments -Wno-missing-field-initializers: YES
Compiler for C supports arguments -Wno-unused-parameter: YES
Compiler for C supports arguments -Wno-unused-result: YES
Compiler for C supports arguments -Wno-format-overflow: NO
Library readline found: YES
Checking for function "rl_replace_line" with dependency -lreadline: YES
Checking if "-Wl,--version-script" links: NO
Compiler for C supports link arguments -Wl,-exported_symbols_list /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/libmrsh.darwin.sym: NO

meson.build:71:1: ERROR: Problem encountered: Linker doesn't support --version-script or -exported_symbols_list

A full log can be found at /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/build/meson-logs/meson-log.txt

@omasanori omasanori force-pushed the mrsh/update-2020-07-27 branch from 7004b98 to 7203ff1 Compare October 31, 2020 20:38
@omasanori
Copy link
Contributor Author

omasanori commented Oct 31, 2020 •
edited
Loading

So now I am trying the opposite, forcing GCC to avoid the problem...
Please test it on macOS again when you have time.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 1, 2020 •
edited
Loading

Result of nixpkgs-review pr 101910 run on x86_64-darwin 1

1 package failed to build:
  • mrsh
Compiler for C supports arguments -Wno-unused-parameter -Wunused-parameter: YES
Compiler for C supports arguments -Wno-unused-result -Wunused-result: YES
Compiler for C supports arguments -Wno-format-overflow -Wformat-overflow: YES
Library readline found: YES
Checking for function "rl_replace_line" with dependency -lreadline: YES
Checking if "-Wl,--version-script" links: NO
Compiler for C supports link arguments -Wl,-exported_symbols_list /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/libmrsh.darwin.sym: NO

meson.build:71:1: ERROR: Problem encountered: Linker doesn't support --version-script or -exported_symbols_list

A full log can be found at /private/tmp/nix-build-mrsh-2020-07-27.drv-0/source/build/meson-logs/meson-log.txt

@omasanori
Copy link
Contributor Author

Thanks a lot! Hmm, these linker options must not be such brand-new ones, so I suspected compilers first. However, it seems that a more detailed analysis is needed...

@SuperSandro2000
Copy link
Member

@omasanori I would be fine with disabling it on darwin for now.

@omasanori
Copy link
Contributor Author

@SuperSandro2000 I guess so. @matthiasbeyer, is it acceptable to you?

@matthiasbeyer
Copy link
Contributor

No objections from me.

@omasanori
mrsh: 2020-01-08 -> 2020-07-27 etc.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
955926a 
- `doCheck` is enabled now.
- It marked as broken on Darwin due to [build failures][macos-issue].

[macos-issue]: NixOS#101910 (comment)

Reference: https://github.com/emersion/mrsh/compare/ef21854fc9..0da902c0ee
Signed-off-by: Masanori Ogino <167209+omasanori@users.noreply.github.com>
@omasanori omasanori force-pushed the mrsh/update-2020-07-27 branch from 7203ff1 to 955926a Compare November 1, 2020 19:47
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/271

@kevincox kevincox merged commit 22409c8 into NixOS:master Nov 2, 2020
@omasanori omasanori deleted the mrsh/update-2020-07-27 branch November 2, 2020 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthiasbeyer matthiasbeyer

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@omasanori @SuperSandro2000 @matthiasbeyer @nixos-discourse @kevincox