search

janet-lang / pkgs

A package listing for Janet.
MIT License
59 stars 25 forks source link

change jurl ref to mine #57

Closed CosmicToast closed 1 year ago

CosmicToast commented 1 year ago

ref: #56 I'm still in the process of finalizing the high level API (just pushed a preview) once that's done I'll finish what's left of the documentation and write the TECH file everything else should roughly be in place as is, and everything is perfectly usable as-is as well

high level API should not be considered stable (syms like http and co) but request should not majorly change anymore (minus obvious improvements like cookies dict and similar)

sogaiu commented 1 year ago

It might make sense to check dependencies of existing projects.

I can work on doing an analysis of the local copies of things I've retrieved.

sogaiu commented 1 year ago

As a sampling of short names used in dependencies, among other things, I came across json, path, uri, sqlite3, jaylib, temple, and, spork.

jurl came up a few times but in full url form (not as a short name) -- though it appears that apart from the one that's the topic of this issue, there are at least 2 others (one looks to be a fork of the other).

FWIW, this is across a bit under 400 repositories.

Possibly one could ping the authors of the packages (as well as the maintainers of the other jurls as they might know of folks using their respective jurls).

Other jurls:

Things using other jurls:

sogaiu commented 1 year ago

I don't know if there's a good place for announcments of this sort, but in a project I look after I have a pinned issue where I announce potential changes. I give some time for feedback before actually going through with the changes.

May be it's not worth having something like that, but thought I would mention the idea.

On a separate note, may be it makes sense to state somewhere (if it isn't already mentioned) that one risks more breakage if specifying a dependency using short names.

CosmicToast commented 1 year ago

As an update, the API should be considered stable now (some minor semantics may change but that’s about it). Documentation is done and 1.0.0 is tagged. Still todo are tests and TECH.md.

CosmicToast commented 1 year ago

Both done. There could always be additional tests, but I think this is ready now. If there are any complaints re: project "tier"-ing ala CONTRIBUTIONS.md, please let me know!

sogaiu commented 1 year ago

I tried to build but so far it hasn't worked for me.

I am using Ubuntu 22.04.2 with the libcurl4-openssl-dev package (7.81.0-1ubuntu1.10_amd64).

I didn't succeed in finding much related to the errors below via search engines. There was one StackOverlfow post that mentioned version issues. I looked for info about which version of curl / libcurl should be used at the jurl README but didn't find any (may be that's not even one of the relevant factors...).

The output I get for jpm build is:

$ jpm build
compiling src/errors.c to build/src___errors.o...
compiling src/util.c to build/src___util.static.o...
compiling src/getinfo.c to build/src___getinfo.o...
compiling src/setopt.c to build/src___setopt.o...
compiling src/jurl.c to build/src___jurl.static.o...
compiling src/callbacks.c to build/src___callbacks.static.o...
compiling src/cleanup.c to build/src___cleanup.static.o...
compiling src/enums.c to build/src___enums.static.o...
compiling src/mime.c to build/src___mime.static.o...
src/getinfo.c:78:10: error: ‘CURLINFO_CAPATH’ undeclared here (not in a function); did you mean ‘CURLOPT_CAPATH’?
   78 |         {CURLINFO_CAPATH,                    "capath",                  JURL_PARAMTYPE_STRING},
      |          ^~~~~~~~~~~~~~~
      |          CURLOPT_CAPATH
src/getinfo.c:79:10: error: ‘CURLINFO_CAINFO’ undeclared here (not in a function); did you mean ‘CURLINFO_CERTINFO’?
   79 |         {CURLINFO_CAINFO,                    "cainfo",                  JURL_PARAMTYPE_STRING},
      |          ^~~~~~~~~~~~~~~
      |          CURLINFO_CERTINFO
src/setopt.c:72:10: error: ‘CURLOPT_PROTOCOLS_STR’ undeclared here (not in a function); did you mean ‘CURLOPT_PROTOCOLS’?
   72 |         {CURLOPT_PROTOCOLS_STR,        "protocols",            JURL_PARAMTYPE_STRING},
      |          ^~~~~~~~~~~~~~~~~~~~~
      |          CURLOPT_PROTOCOLS
src/errors.c:108:10: error: ‘CURLE_UNRECOVERABLE_POLL’ undeclared here (not in a function)
  108 |         {CURLE_UNRECOVERABLE_POLL,       "error/unrecoverable-poll"},
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
error: command failed with non-zero exit code 1
error: command failed with non-zero exit code 1
src/setopt.c:74:10: error: ‘CURLOPT_REDIR_PROTOCOLS_STR’ undeclared here (not in a function); did you mean ‘CURLOPT_REDIR_PROTOCOLS’?
   74 |         {CURLOPT_REDIR_PROTOCOLS_STR,  "redir-protocols",      JURL_PARAMTYPE_STRING},
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |          CURLOPT_REDIR_PROTOCOLS
error: command failed with non-zero exit code 1
CosmicToast commented 1 year ago

This is caused by your curl being fairly old. Curl is currently on version 8.0, while 7.81.0 came out over a year ago (let's not even mention curl 4 :) ). During development, I didn't gate too many things behind version checks (mostly focusing on the versions I happen to have installed across a couple different systems). Since this is being an active problem, let me go and version-buffer those symbols.

Note that this is exactly the kind of output I'm looking for out of build related problems.

Should be fixed now in sha CosmicToast/jurl@09fb489. I spun up a testing fresh ubuntu 22.04 setup and got all of the tests to pass.

In case of similar issues to this one it should be quite trivial to fix them (see: the commit for roughly what it entails in the future).

sogaiu commented 1 year ago

Thanks for taking a look.

This is caused by your curl being fairly old.

Ok.

Curl is currently on version 8.0, while 7.81.0 came out over a year ago (let's not even mention curl 4 :) ).

Sure, it's just the package that is available by default for the latest Ubuntu LTS AFAIU. I'm not a die-hard user of Ubuntu, but I think it's something that's used commonly enough.

The package's version being the age that it is, seems like the typical fate for a Debian-based distribution.

During development, I didn't gate too many things behind version checks (mostly focusing on the versions I happen to have installed across a couple different systems).

Thanks for the explanation. What do you think of the idea of stating which version (or versions) of libcurl you know should work in the README (or some other place linked from it)? May be this is already somewhere that I missed.

I looked at the build errors section of your CONTRIBUTING document and thought that for someone to figure out where a problem might be with their setup, knowing what versions of things should be in place might be helpful information.

Since this is being an active problem, let me go and version-buffer those symbols.

That seems to have worked. Building and testing work here now, yay :)

In case of similar issues to this one it should be quite trivial to fix them (see: the commit for roughly what it entails in the future).

Sounds good.

CosmicToast commented 1 year ago

The package's version being the age that it is, seems like the typical fate for a Debian-based distribution.

Sadly yeah. I have the distro developer brains (I've been rather involved with fedora, arch, gentoo, alpine, and a few others), so I'm not a good judge of these things. Still, I'm very much willing to add the checks whenever, it's not a big task. Adding checks around every single symbol would be a big task, however, so I'd rather take it piece by piece.

Thanks for the explanation. What do you think of the idea of stating which version (or versions) of libcurl you know should work in the README (or some other place linked from it)? May be this is already somewhere that I missed.

Sounds like a good idea. I added it to the README's caveats section in the latest commit.

sogaiu commented 1 year ago

Thanks for adding the version info :)

pepe commented 1 year ago

Thank you both :-). At least, I can now deprecate shriek for good :-).

If @bakpakin does not object, I will merge brand-new jurl later today or tomorrow.