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

Add support for libgda on darwin #71200

Merged
merged 3 commits into from Oct 16, 2019
Merged

Add support for libgda on darwin #71200

merged 3 commits into from Oct 16, 2019

Conversation

lavoiesl
Copy link
Contributor

@lavoiesl lavoiesl commented Oct 15, 2019 •
edited

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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/) 
     $ /nix/store/b3sddkvy7amam6rmr4cbf423rwazm5rl-libgda-5.2.9/bin/gda-sql
 gda> .lp
        Installed providers list
 Provider  │ Description
 ──────────┼──────────────────────────────
 MySQL     │ Provider for MySQL databases
 SQLCipher │ Provider for SQLCipher
 SQLite    │ Provider for SQLite databases
  • 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.
Notify maintainers

cc @jtojnar

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 8.has: package (new) labels Oct 15, 2019
@hedning
Copy link
Contributor

hedning commented Oct 15, 2019

@GrahamcOfBorg build libgda

@lavoiesl
Copy link
Contributor Author

It's my first time contributing, is this PR missing anything before it can be merged?

I’m not entirely sure what it required from the checklist template.

I couldn't run nix-shell -p nix-review --run "nix-review wip" and I didn't figure out how to run nix path-info -S

hedning
hedning approved these changes Oct 16, 2019
View reviewed changes
Copy link
Contributor

@hedning hedning left a comment

Choose a reason for hiding this comment

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

LGTM at least.

@worldofpeace
Copy link
Contributor

It's my first time contributing, is this PR missing anything before it can be merged?

I’m not entirely sure what it required from the checklist template.

I couldn't run nix-shell -p nix-review --run "nix-review wip" and I didn't figure out how to run nix path-info -S

I'd say using the archive link from jtojnar is important.

worldofpeace
worldofpeace suggested changes Oct 16, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

see @jtojnar's comment

@lavoiesl
gnome3.libgda: explicitly enable or disable mysql and postgres
ce5616f 
This prevents an issue where mysql and postgres support can be
auto-detected by the configure scripts, resulting in a broken build or a
build that cannot be ran on a different computer.
@lavoiesl
gnome3.libgda: use embedded sqlite support on macOS
9b83a23 
Taken from the Homebrew Formula
@lavoiesl
gnome3.libgda: enable darwin builds
adc635f
@lavoiesl
Copy link
Contributor Author

My bad, I read that as a footnote, not a request for adding a comment 😄

Added 👍

@worldofpeace worldofpeace merged commit 6695d3f into NixOS:master Oct 16, 2019
@worldofpeace
Copy link
Contributor

Thanks for contributing @lavoiesl 🌸

I’m not entirely sure what it required from the checklist template.
I couldn't run nix-shell -p nix-review --run "nix-review wip" and I didn't figure out how to run nix path-info -S

None of the things are required to submit a PR, it's mostly a checklist of info for the reviewers.
Only minimum thing for things to be considered is if they fit https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md. It's encouraged to check whatever you believe you've done or is valid.

As for nix path-info -S, you should give it the result symlink for what was built

nix path-info -S ./result

and it's only really useful info if you maybe added a dependency. And in this case I would have been interested in checking that changes for darwin don't have effects on linux.
As you've built it on darwin, I don't think you could've produced that info.

@lavoiesl lavoiesl mentioned this pull request Nov 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace Awaiting requested review from worldofpeace

@jtojnar jtojnar Awaiting requested review from jtojnar

@hedning hedning Awaiting requested review from hedning

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lavoiesl @hedning @worldofpeace @jtojnar @Janik-Haag