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

falcon: 2013-09-19 -> unstable-2018-10-23 #108040

Merged
merged 1 commit into from May 10, 2021
Merged

falcon: 2013-09-19 -> unstable-2018-10-23 #108040

merged 1 commit into from May 10, 2021

Conversation

siraben
Copy link
Member

@siraben siraben commented Dec 31, 2020 •
edited

Motivation for this change
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.

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

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

1 package built:
  • falcon

@ofborg ofborg bot requested a review from pSub December 31, 2020 12:53
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Dec 31, 2020 •
edited

This is a semi-automatic executed nixpkgs-review. If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

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

1 package built:
  • falcon

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

falcon.log:

gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

Near pkgs/development/interpreters/falcon/default.nix:19:5:

   |
19 |     license = licenses.gpl2;
   |     ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/unclear-gpl.md

@davidak
Copy link
Member

davidak commented Feb 21, 2021

@siraben can you clarify the license as the comment from @SuperSandro2000 tells?

@prusnak
Copy link
Member

prusnak commented Feb 21, 2021 •
edited

According to https://github.com/falconpl/falcon/blob/637e2d5cd950a874496042993c02ab7d17c1b688/LICENSE#L286-L289 the license should be licenses.gpl2Only

@siraben siraben force-pushed the falcon-update branch 2 times, most recently from b533c7c to 4dfeeba Compare February 21, 2021 13:10
@prusnak
Copy link
Member

prusnak commented Feb 21, 2021 •
edited

The build fails on x86_64-linux - https://logs.nix.ci/?key=nixos/nixpkgs.108040&attempt_id=878676a0-3d64-47d4-98a7-534a4839741c

/build/source/engine/stringstream.cpp:204:13: error: narrowing conversion of '-1' from 'int' to 'long long unsigned int' [https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wnarrowing-Wnarrowing]
  204 |       case -1: description = "Out of Memory"; return true;

@siraben
Copy link
Member Author

siraben commented Feb 21, 2021 •
edited

Interesting, the build succeeded for both me and @SuperSandro2000 on x86_64-linux on 2020-12-31.

What should I adjust the hardening flags to?

@davidak
Copy link
Member

davidak commented Feb 21, 2021 •
edited

Result of nixpkgs-review pr 108040 1

1 package failed to build:
  • falcon

Same issue:

builder for '/nix/store/5i8sb1dn40mz9fagsmc882l6b18jkqpl-falcon-unstable-2018-10-23.drv' failed with exit code 2; last 10 log lines:
  [ 24%] Building CXX object engine/CMakeFiles/falcon_engine.dir/symtab.cpp.o
  [ 25%] Building CXX object engine/CMakeFiles/falcon_engine.dir/syntree.cpp.o
  /build/source/engine/stringstream.cpp: In member function 'virtual bool Falcon::StringStream::errorDescription(Falcon::String&) const':
  /build/source/engine/stringstream.cpp:204:13: error: narrowing conversion of '-1' from 'int' to 'long long unsigned int' [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wnarrowing-Wnarrowing8;;]
    204 |       case -1: description = "Out of Memory"; return true;
        |             ^
  make[2]: *** [engine/CMakeFiles/falcon_engine.dir/build.make:1057: engine/CMakeFiles/falcon_engine.dir/stringstream.cpp.o] Error 1
  make[2]: *** Waiting for unfinished jobs....
  make[1]: *** [CMakeFiles/Makefile2:1065: engine/CMakeFiles/falcon_engine.dir/all] Error 2
  make: *** [Makefile:149: all] Error 2

@SuperSandro2000
Copy link
Member

What should I adjust the hardening flags to?

Probably the issue is similar to the packages listed here https://search.nix.gsc.io/?q=narrowing&i=nope&files=&repos=

@LeSuisse LeSuisse mentioned this pull request May 8, 2021
Merged
10 tasks
@siraben
Copy link
Member Author

siraben commented May 9, 2021

@SuperSandro2000 @prusnak @davidak please take another look

@siraben
Copy link
Member Author

siraben commented May 9, 2021

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

1 package built:
  • falcon

@davidak
Copy link
Member

davidak commented May 9, 2021

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

1 package built:
  • falcon

davidak
davidak approved these changes May 9, 2021
View reviewed changes
Copy link
Member

@davidak davidak left a comment

Choose a reason for hiding this comment

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

Change looks OK

Builds, seem to work

[nix-shell:~/.cache/nixpkgs-review/pr-108040-1]$ falcon -v
Falcon compiler and interpreter.
Version 0.9.6.9 (Chimera)

@davidak davidak merged commit 71bc0ca into NixOS:master May 10, 2021
@siraben siraben deleted the falcon-update branch May 10, 2021 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prusnak prusnak prusnak approved these changes

@davidak davidak davidak approved these changes

@pSub pSub Awaiting requested review from pSub

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

Assignees
No one assigned
Labels
8.has: package (new) 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
4 participants
@siraben @SuperSandro2000 @davidak @prusnak