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

libfetchers/github: allow `url` attribute #4035

Merged
merged 2 commits into from Sep 21, 2020
Merged

libfetchers/github: allow `url` attribute #4035

merged 2 commits into from Sep 21, 2020

Conversation

@Ma27
Copy link
Member

@Ma27 Ma27 commented Sep 18, 2020

Since 108debe we allow a
url-attribute for the github-fetcher to fetch tarballs from
self-hosted gitlab/github instances.

However it's not used when defining e.g. a flake-input

foobar = {
    type = "github";
    url = "gitlab.myserver";
    /* ... */
}

and breaks with an evaluation-error:

error: --- Error --------------------------------------nix
unsupported input attribute 'url'
(use '--show-trace' to show detailed location information)

This patch allows flake-inputs to be fetched from self-hosted instances
as well.

Since 108debe we allow a
`url`-attribute for the `github`-fetcher to fetch tarballs from
self-hosted `gitlab`/`github` instances.

However it's not used when defining e.g. a flake-input

    foobar = {
        type = "github";
        url = "gitlab.myserver";
        /* ... */
    }

and breaks with an evaluation-error:

    error: --- Error --------------------------------------nix
    unsupported input attribute 'url'
    (use '--show-trace' to show detailed location information)

This patch allows flake-inputs to be fetched from self-hosted instances
as well.
@edolstra
Copy link
Member

@edolstra edolstra commented Sep 21, 2020

url seems wrong since gitlab.server is not a valid URL. Maybe it should be server or host or something like that?

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 21, 2020

@edolstra you're obviously right and I'm fine with a rename. I'd just like to point out that I used the name-decision that was originally taken in 108debe.

I think host is better. If you agree with this I'll update my PR accordingly.

@edolstra
Copy link
Member

@edolstra edolstra commented Sep 21, 2020

Ah, sorry about that. Yes please do!

@Ma27
Copy link
Member Author

@Ma27 Ma27 commented Sep 21, 2020

@edolstra fixed. Please note that this needs to be fixed as well now in https://github.com/edolstra/flake-compat/blob/master/default.nix :)

@edolstra edolstra merged commit cbe0bb2 into NixOS:master Sep 21, 2020
2 checks passed
2 checks passed
tests (ubuntu-latest)
Details
tests (macos-latest)
Details
@Ma27 Ma27 deleted the Ma27:url-attr branch Sep 21, 2020
@zanculmarktum

This comment has been minimized.

Copy link

@zanculmarktum zanculmarktum commented on 56f1e0d Sep 23, 2020

Before this commit, wasn't url supposed to be a complete url of the inputs (i.e. https://github.com/user/repo/path/to/somewhere), not just the host (i.e. github.com)? 🤔

Or did I misunderstood it?

This comment has been minimized.

Copy link
Member

@cole-h cole-h replied Sep 23, 2020

According to 56f1e0d#diff-597785e0a47d7f86a1982d22662296afR213, I would disagree.

This comment has been minimized.

Copy link

@zanculmarktum zanculmarktum replied Sep 23, 2020

Is this url the same url specified in the flake.lock file?

e.g.

{
  "nodes": {
    "nixpkgs": {
      "locked": {
        "narHash": "sha256-J6bDPuMkEsB/aujzqEN3GWOLNf/6xNhyR9SubLzCxiQ=",
        "type": "tarball",
        "url": "https://releases.nixos.org/nixos/20.03/nixos-20.03.2802.f8a10a77193/nixexprs.tar.xz"
      },
    }
  }
}

This comment has been minimized.

Copy link
Member Author

@Ma27 Ma27 replied Sep 23, 2020

This is about type = "git{hub,lab}";, NOT about tarballs.

This comment has been minimized.

Copy link

@zanculmarktum zanculmarktum replied Sep 23, 2020

Oh right, the type = github by default doesn't use url.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.