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

csvs-to-sqlite: linking click version 7 explicitly to pass build #137442

Merged
merged 2 commits into from
Sep 14, 2021

Conversation

asbachb
Copy link
Contributor

@asbachb asbachb commented Sep 11, 2021

The fix was copied from watson.

Motivation for this change

The application is requesting click in version 7.

related: simonw/csvs-to-sqlite#80

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
 asbachb@nixo-t14s  ~/dev/src/nix/nixpkgs   fixup/csvs-to-sqlite  nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
$ git worktree add /home/asbachb/.cache/nixpkgs-review/rev-009e1287b54111b31a60ad1e9cb530aab762c72a-dirty-1/nixpkgs 9f761047cc4d3a7d6b9bbf498fd4f93864e9b311
Preparing worktree (detached HEAD 9f761047cc4)
Updating files: 100% (27778/27778), done.
HEAD is now at 9f761047cc4 Merge pull request #137425 from SuperSandro2000/typora
$ nix-env --option system x86_64-linux -f /home/asbachb/.cache/nixpkgs-review/rev-009e1287b54111b31a60ad1e9cb530aab762c72a-dirty-1/nixpkgs -qaP --xml --out-path --show-trace
No diff detected, stopping review...
$ git worktree prune
  • Tested execution of all binary files (usually in ./result/bin/)
 ✘ asbachb@nixo-t14s  ~/dev/src/nix/nixpkgs/result/bin  ./csvs-to-sqlite --help
Usage: csvs-to-sqlite [OPTIONS] PATHS... DBNAME

  PATHS: paths to individual .csv files or to directories containing .csvs

  DBNAME: name of the SQLite database file to create

Options:
  -s, --separator TEXT         Field separator in input .csv
  -q, --quoting INTEGER        Control field quoting behavior per csv.QUOTE_*
                               constants. Use one of QUOTE_MINIMAL (0),
                               QUOTE_ALL (1), QUOTE_NONNUMERIC (2) or
                               QUOTE_NONE (3).

  --skip-errors                Skip lines with too many fields instead of
                               stopping the import

  --replace-tables             Replace tables if they already exist
  -t, --table TEXT             Table to use (instead of using CSV filename)
  -c, --extract-column TEXT    One or more columns to 'extract' into a
                               separate lookup table. If you pass a simple
                               column name that column will be replaced with
                               integer foreign key references to a new table
                               of that name. You can customize the name of the
                               table like so:     state:States:state_name
                               
                               This will pull unique values from the 'state'
                               column and use them to populate a new 'States'
                               table, with an id column primary key and a
                               state_name column containing the strings from
                               the original column.

  -d, --date TEXT              One or more columns to parse into ISO formatted
                               dates

  -dt, --datetime TEXT         One or more columns to parse into ISO formatted
                               datetimes

  -df, --datetime-format TEXT  One or more custom date format strings to try
                               when parsing dates/datetimes

  -pk, --primary-key TEXT      One or more columns to use as the primary key
  -f, --fts TEXT               One or more columns to use to populate a full-
                               text index

  -i, --index TEXT             Add index on this column (or a compound index
                               with -i col1,col2)

  --shape TEXT                 Custom shape for the DB table - format is
                               csvcol:dbcol(TYPE),...

  --filename-column TEXT       Add a column with this name and populate with
                               CSV file name

  --no-index-fks               Skip adding index to foreign key columns
                               created using --extract-column (default is to
                               add them)

  --no-fulltext-fks            Skip adding full-text index on values extracted
                               using --extract-column (default is to add them)

  --just-strings               Import all columns as text strings by default
                               (and, if specified, still obey --shape,
                               --date/datetime, and --datetime-format)

  --version                    Show the version and exit.
  --help                       Show this message and exit.

@SuperSandro2000
Copy link
Member

We could only use this approach if csvs-to-sqlite would be no python package.

/cc @mweinelt @dotlambda

@asbachb asbachb marked this pull request as draft September 11, 2021 22:27
@mweinelt
Copy link
Member

We don't allow these kinds of overwrites for libraries, as they propagate those dependencies, which could lead to collisions.

@dotlambda dotlambda closed this Sep 11, 2021
@dotlambda dotlambda reopened this Sep 11, 2021
@dotlambda
Copy link
Member

We could only use this approach if csvs-to-sqlite would be no python package.

Yes, there's no reason for this to be in pythonPackages.

However, watson isn't doing this well at all. I'm making a PR to improve it.

@r-rmcgibbo
Copy link

r-rmcgibbo commented Sep 12, 2021

Result of nixpkgs-review pr 137442 at 009e1287 run on aarch64-linux 1

2 packages built successfully:
  • csvs-to-sqlite (python39Packages.csvs-to-sqlite)
  • python38Packages.csvs-to-sqlite

Result of nixpkgs-review pr 137442 at d522e137 run on x86_64-linux 1

1 package built successfully:
  • csvs-to-sqlite

@asbachb
Copy link
Contributor Author

asbachb commented Sep 12, 2021

I removed the package from pythonPackages and added an override in all-packages. Maybe you can have another look.

@dotlambda
Copy link
Member

However, watson isn't doing this well at all. I'm making a PR to improve it.

#137552

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Use packageOverrides, like I did for watson.

Also actually move the expression to a different directory.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@github-actions github-actions bot added 6.topic: erlang 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde 6.topic: vim 8.has: changelog labels Sep 14, 2021
@github-actions github-actions bot added 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 14, 2021
@asbachb
Copy link
Contributor Author

asbachb commented Sep 14, 2021

@dotlambda Actually I moved the package to another location. But I'm unable to apply the patches to that package as you did in watson. If I apply these I just get an error: error: anonymous function at /home/asbachb/dev/src/nix/nixpkgs/pkgs/tools/misc/csvs-to-sqlite/default.nix:1:1 called without required argument 'buildPythonPackage'.

@github-actions github-actions bot removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: erlang 8.has: documentation 6.topic: qt/kde 8.has: changelog 6.topic: vim 6.topic: kernel The Linux kernel labels Sep 14, 2021
@dotlambda
Copy link
Member

Actually I moved the package to another location.

Oops, sorry.

But I'm unable to apply the patches to that package as you did in watson. If I apply these I just get an error: error: anonymous function at /home/asbachb/dev/src/nix/nixpkgs/pkgs/tools/misc/csvs-to-sqlite/default.nix:1:1 called without required argument 'buildPythonPackage'.

The new expression's only arguments should be lib, python3, and fetchFromGitHub.

…in order to fix build

The application is not capable to build with `click` greate than version `7`.
@dotlambda dotlambda merged commit b2488e1 into NixOS:staging-next Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants