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

Support multiple registries and repositories #248

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

nouwaarom
Copy link
Contributor

@nouwaarom nouwaarom commented Mar 25, 2021

Towards fixing #105
In order to support multiple registries and be able to download packages from multiple sources I had to change quite a few things.

  • An abstraction for registries so that multiple registries can be added. (currently github wiki pages and gitlab files are supported)
  • Add extra registries in clib.json in a registries item.
  • Store secrets in clib_secrets.json so that we can connect with private registries and repositories
  • Make an abstraction for repositories, a repository is a place we can download packages from (currently implemented gitlab and github)

Changes

  • Changed the "has pthread?" test because it doesn't work when cross-compiling with dockcross.
  • Move instructions for building from Readme.md to Building.md and add instructions for cross-compiling for windows (as it took me some time to figure this out)
  • Changed the windows test build in github actions to use pthread as that seems to work fine (after fixing the pthread detection script)
  • Add instructions for how to install from other sources to Usage.md
  • Fix multithreading when downloading package files.

Todo

  • Add a test for reading and using secrets from clib_secrets.json
  • Add a test for reading packages from an extra registry.
  • Add a test for installing a package from gitlab.com

@jwerle
Copy link
Member

jwerle commented Mar 26, 2021

@nouwaarom i can give this a try this afternoon (UTC-4) - I do not know of any clib packages hosted on Gitlab, do you have some instructions that I can use to try this stuff out?

@nouwaarom
Copy link
Contributor Author

Okay, cool. I have been using a private gitlab instance to develop this. I will setup a small demo on gitlab.com now and add some instructions.

@nouwaarom
Copy link
Contributor Author

I create a small demo. It can be found at https://gitlab.com/nouwaarom/clib-demo with some instructions in the readme.
I have developed on ubuntu. I don't know what system you use but I hope it compiles ;)
Please let me know if you have questions or issues.

@stephenmathieson
Copy link
Member

This is great! Fantastic work @nouwaarom! Thank you for putting this together.

A PR this big may take some time to review, so please be patient with us. I will try to spend some time testing this out in the next few days.

src/registry/registry-manager.c Outdated Show resolved Hide resolved
src/registry/registry-manager.h Outdated Show resolved Hide resolved
src/clib-install.c Outdated Show resolved Hide resolved
@jwerle
Copy link
Member

jwerle commented Mar 27, 2021

I am very excited for this! I am working through the changes locally now and have some initial notes:

running clib install . on the clib-demo project root before moving clib_secrets.json.dist to clib_secrets.json results in a segfault:

 $ clib install .                                                                                                                                                           
       error : Secrets file clib_secrets.json does not exist.
        info : reading local clib.json
Segmentation fault (core dumped)

after moving the file, if I run, clib install . before setting my personal access token in the file I get:

 $ clib install .                                                                                                                                                           
        info : reading local clib.json
REGISTRY: could not list packages from. https://gitlab.com/api/v4/projects/25447829/repository/files/README.md/raw?ref=master
     warning : missing "repo" in  clib.json
     warning : missing repo in clib.json or package.json file for clib-demo
       error : no such file: package.json

we should probably have more helpful error messages

after setting the personal access token in the clibs_secrets.json file I was able to successful install the dependencies

I echo @stephenmathieson on being patient as this is a big change. Perhaps we can get some tests in this change too?

Copy link
Member

@diasbruno diasbruno left a comment

Choose a reason for hiding this comment

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

Hope these suggestions make sense.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
src/registry/registry.c Show resolved Hide resolved
src/repository/gitlab-repository.c Show resolved Hide resolved
src/repository/gitlab-repository.c Outdated Show resolved Hide resolved
src/registry/github-registry.c Show resolved Hide resolved
src/registry/github-registry.c Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@nouwaarom
Copy link
Contributor Author

I echo @stephenmathieson on being patient as this is a big change. Perhaps we can get some tests in this change too?

Yes, I think that adding some tests is a good idea. I am currently working on passing all current tests again because most of them still fail :) After that I will add some tests for the new functionality. What is the current strategy for writing tests?

@nouwaarom nouwaarom force-pushed the feature/multiple-registries branch 4 times, most recently from 754ef31 to 0ba5693 Compare April 2, 2021 14:41
src/clib-update.c Outdated Show resolved Hide resolved
src/clib-upgrade.c Outdated Show resolved Hide resolved
src/clib-install.c Outdated Show resolved Hide resolved
Copy link
Member

@stephenmathieson stephenmathieson left a comment

Choose a reason for hiding this comment

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

Sorry I haven't been more active here. Been super busy with my 9-5.

I just pulled this branch down and poked around a bit. I got a seg fault doing:

make
./clib-install sha1

Unfortunately I cleared my terminal buffer, so I don't have the logs 🤷

I then tried without cache and got an error:

/clib-install -c sha1
       fetch : clibs/sha1:package.json
       error : unable to fetch https://raw.githubusercontent.com/clibs/sha1/0.0.1/sha1.h
       error : unable to fetch https://raw.githubusercontent.com/clibs/sha1/0.0.1/sha1.c
➜  clib git:(feature/multiple-registries) ✗

I figured it might be something with the sha1 package, so I tried another:

➜  clib git:(feature/multiple-registries) ✗ ./clib-install module
       error : Unable to install package module
➜  clib git:(feature/multiple-registries) ✗

I'm not sure what's up, but I am unable to install anything using this branch.


I'm on macOS btw, with:

cc -v
Apple clang version 12.0.0 (clang-1200.0.32.29)
Target: x86_64-apple-darwin20.3.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

@nouwaarom
Copy link
Contributor Author

Hi Stephen. I'm sorry to hear that you could not install those dependencies.
The problem with sha1 is that package.json in master specifies that the latest version is 0.0.1 but this tag is missing. (I think we should add an error message saying the last tag is missing.

The problem with module is that it is not listed in a registry. The registry is required to lookup where a package can be downloaded from. (gitlab or github). We could add a fallback to github.com but it is better to just add module to the registry IMHO.

@nouwaarom nouwaarom changed the title WIP: Support multiple registries and repositories Support multiple registries and repositories May 27, 2021
@nouwaarom nouwaarom force-pushed the feature/multiple-registries branch from 43e2af2 to 6ce2c5a Compare July 28, 2021 08:56
@nouwaarom nouwaarom force-pushed the feature/multiple-registries branch 2 times, most recently from dff7bd8 to 8de9c05 Compare July 28, 2021 09:05
@nouwaarom nouwaarom force-pushed the feature/multiple-registries branch 2 times, most recently from 8aa517e to fca0528 Compare December 26, 2021 13:59
nouwaarom and others added 15 commits December 28, 2021 23:55
clib_secrets.json

misc: Cleanup, add headers.
misc: Fix some problems codacy detected.
…bug with downloading packages from gitlab, move common definitinos to clib-settings
misc: Apply suggestions from code review

Co-authored-by: Bruno Dias <dias.h.bruno@gmail.com>
public gitlab repositories

misc: Cleanup, download from the correct version when downloading from gitlab.
…are properly joined. Fixed some compiler warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants