Fix wrong `go get` path in README and change module name for `go-elektra`

tmakar commented 1 year ago

Basics

[ ] Short descriptions of your changes are in the release notes (added as entry in doc/news/_preparation_next_release.md which contains _(my name)_) Please always add them to the release notes.
[ ] Details of what you changed are in commit messages (first line should have module: short statement syntax)
[ ] References to issues, e.g. close #X, are in the commit messages.
[ ] The buildservers are happy. If not, fix in this order:
- add a line in doc/news/_preparation_next_release.md
- reformat the code with scripts/dev/reformat-all
- make all unit tests pass
- fix all memleaks
- fix the CI itself (or rebase if already fixed)
[ ] The PR is rebased with current master.

Checklist

[ ] I added unit tests for my code
[ ] I fully described what my PR does in the documentation (not in the PR description)
[ ] I fixed all affected documentation (see Documentation Guidelines)
[ ] I fixed all affected decisions (see Decision Process)
[ ] I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
[ ] I updated all meta data (e.g. README.md of plugins and METADATA.ini)
[ ] I mentioned every code not directly written by me in reuse syntax

Review

[ ] Documentation is introductory, concise, good to read and describes everything what the PR does
[ ] Examples are well chosen and understandable
[ ] Code is conforming to our Coding Guidelines
[ ] APIs are conforming to our Design Guidelines
[ ] Code is consistent to our Design Decisions

Labels

[ ] Add the "work in progress" label if you do not want the PR to be reviewed yet.
[ ] Add the "ready to merge" label if everything is done and no further pushes are planned by you.

tmakar commented 1 year ago

If using go.libelektra.org in the module name explicitly it seems not work and does not the go binding at all, therefore I have changed to module to match the exact repo path.

NOTE: go get should still work with go.libelektra.org due to the vanity url metatag go-import.

markus2330 commented 1 year ago

If using go.libelektra.org in the module name explicitly it seems not work and does not the go binding at all, therefore I have changed to module to match the exact repo path.

I don't understand what you want to say here. There is no "exact repo" path, with a proper setup, "go.libelektra.org" should work too. And we would have the advantage that we can migrate from GitHub if needed.

tmakar commented 1 year ago

Well, it unfortunately does not find the module.

go get go.libelektra.org/src/bindings/go-elektra@a59fc7986097c11104c89c822ca5d650968ce83e
go: module go.libelektra.org@a59fc7986097c11104c89c822ca5d650968ce83e found (v0.0.0-20230517052431-a59fc7986097), but does not contain package go.libelektra.org/src/bindings/go-elektra

Maybe I have missed something but I did a similar nested repo (https://github.com/tmakar/test-go) yesterday and tested the go get for it and it worked. The only difference was, that I was not using go-import metatag.

@kodebach Have you any ideas here?

tmakar commented 1 year ago

jenkins build libelektra please

kodebach commented 1 year ago

I tried a lot of things (go get -x and go mod download -x produce detailed logs, if you do go clean -modcache before to get all steps), but I not sure I understand exactly why it doesn't work.

All I can say for certain is that with the current file:

go downloads this file https://proxy.golang.org/go.libelektra.org/@v/v0.0.0-20230517052431-a59fc7986097.zip, which is a copy of the correct commit, but with all go modules stripped out. (Note: If you use GOPROXY=direct to avoid the proxy you get a bit more infos about what the proxy does, in that case go builds the ZIP file from a bare git repo clone)
You can do go get github.com/ElektraInitiative/libelektra/src/bindings/go-elektra@a59fc7986097c11104c89c822ca5d650968ce83e, which downloads all the correct stuff but then complains that the module declares it's path as go.libelektra.org/... not github.com/...
The proxy only has data for https://proxy.golang.org/go.libelektra.org/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info, but not for https://proxy.golang.org/go.libelektra.org/src/bindings/go-elektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info. But with the direct github.com URL both https://proxy.golang.org/github.com/elektrainitiative/libelektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info and https://proxy.golang.org/github.com/elektrainitiative/libelektra/src/bindings/go-elektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info exist.
With https://proxy.golang.org/github.com/%21elektra%21initiative/libelektra/@v/v0.0.0-20230517052431-a59fc7986097.zip you also get the same broken ZIP file as go downloads. But here it you can download https://proxy.golang.org/github.com/%21elektra%21initiative/libelektra/src/bindings/go-elektra/@v/v0.0.0-20230517052431-a59fc7986097.zip, which does have the correct files. (Note: The %21 is some sort of escape mechanism for the uppercase letters. It also works if you remove the %21, since GitHub isn't actually case sensitive).

My best guess is that we should change things so that:

https://go.libelektra.org should return a 404

https://go.libelektra.org/src/bindings/go-elektra should return

 <!DOCTYPE html>
 <html lang="en">
   <head>
     <meta charset="utf-8" />
     <meta
       name="go-import"
       content="go.libelektra.org git github.com/ElektraInitiative/libelektra"
     />
   </head>
   <body></body>
 </html>

Note: the https:// isn't needed and without it go also tries ssh if https fails.

Nothing more is needed, because elektrad while it is a go module, should not be used as a dependency by others.

I hope go interprets this as: Replace go.libelektra.org with github.com/ElektraInitiative/libelektra in the path where you got this file from. I.e. it when loading go.libelektra.org/src/bindings/go-elektra, it switches to github.com/ElektraInitiative/libelektra/src/bindings/go-elektra and everything works out. I think that doesn't happen right now, because it get's the go-import stuff only from go.libelektra.org not from go.libelektra.org/src/bindings/go-elektra.

However, one thing I'm 99% sure about: Changing the module declarations in the repo as this PR does won't fix anything. That'll just lead to go complaining about the declared path not matching the path used to download. (happens if you do go get github.com/ElektraInitiative/libelektra/src/bindings/go-elektra@a59fc7986097c11104c89c822ca5d650968ce83e right now)

tmakar commented 1 year ago

I tried a lot of things (go get -x and go mod download -x produce detailed logs, if you do go clean -modcache before to get all steps), but I not sure I understand exactly why it doesn't work.

All I can say for certain is that with the current file:
* go downloads this file https://proxy.golang.org/go.libelektra.org/@v/v0.0.0-20230517052431-a59fc7986097.zip, which is a copy of the correct commit, but with all go modules stripped out. (Note: If you use `GOPROXY=direct` to avoid the proxy you get a bit more infos about what the proxy does, in that case go builds the ZIP file from a bare git repo clone)

* You can do `go get github.com/ElektraInitiative/libelektra/src/bindings/go-elektra@a59fc7986097c11104c89c822ca5d650968ce83e`, which downloads all the correct stuff but then complains that the module declares it's path as `go.libelektra.org/...` not `github.com/...`

* The proxy only has data for https://proxy.golang.org/go.libelektra.org/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info, but not for https://proxy.golang.org/go.libelektra.org/src/bindings/go-elektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info. But with the direct github.com URL both https://proxy.golang.org/github.com/elektrainitiative/libelektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info and https://proxy.golang.org/github.com/elektrainitiative/libelektra/src/bindings/go-elektra/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info exist.

* With https://proxy.golang.org/github.com/%21elektra%21initiative/libelektra/@v/v0.0.0-20230517052431-a59fc7986097.zip you also get the same broken ZIP file as go downloads. But here it you can download https://proxy.golang.org/github.com/%21elektra%21initiative/libelektra/src/bindings/go-elektra/@v/v0.0.0-20230517052431-a59fc7986097.zip, which does have the correct files. (Note: The `%21` is some sort of escape mechanism for the uppercase letters. It also works if you remove the `%21`, since GitHub isn't actually case sensitive).
My best guess is that we should change things so that:
1. `https://go.libelektra.org` should return a 404

2. `https://go.libelektra.org/src/bindings/go-elektra` should return
   ```
   <!DOCTYPE html>
   <html lang="en">
     <head>
       <meta charset="utf-8" />
       <meta
         name="go-import"
         content="go.libelektra.org git github.com/ElektraInitiative/libelektra"
       />
     </head>
     <body></body>
   </html>
   ```

   Note: the `https://` isn't needed and without it go also tries ssh if https fails.

3. Nothing more is needed, because `elektrad` while it is a go module, should not be used as a dependency by others.
I hope go interprets this as: Replace go.libelektra.org with github.com/ElektraInitiative/libelektra in the path where you got this file from. I.e. it when loading go.libelektra.org/src/bindings/go-elektra, it switches to github.com/ElektraInitiative/libelektra/src/bindings/go-elektra and everything works out. I think that doesn't happen right now, because it get's the go-import stuff only from go.libelektra.org not from go.libelektra.org/src/bindings/go-elektra.

However, one thing I'm 99% sure about: Changing the module declarations in the repo as this PR does won't fix anything. That'll just lead to go complaining about the declared path not matching the path used to download. (happens if you do go get github.com/ElektraInitiative/libelektra/src/bindings/go-elektra@a59fc7986097c11104c89c822ca5d650968ce83e right now)

Yes, I also tried a lot the last couple of weeks :sweat_smile: I think your guesses might work out. Lets give them a try, we can't lose anyway, can only gain more insight into this "blackbox".

@markus2330 Please make sure that the following things work (I assume only you can do them):

https://go.libelektra.org should return a 404
https://go.libelektra.org/src/bindings/go-elektra should return

<!DOCTYPE html>

Note: the https:// isn't needed and without it go also tries ssh if https fails.

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta
      name="go-import"
      content="go.libelektra.org git github.com/ElektraInitiative/libelektra"
    />
  </head>
  <body></body>
</html>

tmakar commented 1 year ago

@markus2330 @kodebach I will adapt the README in this PR, I found one thing which is wrong.

tmakar commented 1 year ago

jenkins build libelektra please

markus2330 commented 1 year ago

I changed it, is 403 for curl https://go.libelektra.org also ok?

markus2330 commented 1 year ago

Probably cannot do anything else today. Lukas can also change it, it is on /srv/go/ of the community server.

markus2330 commented 1 year ago

@lukashartl I changed go.libelektra.org-le-ssl.conf (removed Indexes), is this already generated by Ansible?

kodebach commented 1 year ago

403 should be fine too. AFAIK go only processes 200 responses and ignores everything else.

tmakar commented 1 year ago

Probably cannot do anything else today. Lukas can also change it, it is on /srv/go/ of the community server.

Thank you! Let me know when it is ready!

markus2330 commented 1 year ago

I already changed it yesterday as described in https://go.libelektra.org/src/bindings/go-elektra. But it doesn't work for me:

go get go.libelektra.org/src/bindings/go-elektra@371d3a5cf04f5addc9eb1160454b2d09f211b765
go: go.libelektra.org/src/bindings/go-elektra@371d3a5cf04f5addc9eb1160454b2d09f211b765: unrecognized import path "go.libelektra.org/src/bindings/go-elektra": reading https://go.libelektra.org/?go-get=1: 403 Forbidden

Please advice which changes are needed.

tmakar commented 1 year ago

I already changed it yesterday as described in https://go.libelektra.org/src/bindings/go-elektra. But it doesn't work for me:
go get go.libelektra.org/src/bindings/go-elektra@371d3a5cf04f5addc9eb1160454b2d09f211b765
go: go.libelektra.org/src/bindings/go-elektra@371d3a5cf04f5addc9eb1160454b2d09f211b765: unrecognized import path "go.libelektra.org/src/bindings/go-elektra": reading https://go.libelektra.org/?go-get=1: 403 Forbidden
Please advice which changes are needed.

Oh sorry, maybe I didn‘t write it explicitly, I was talking about letting me know when this is also finished, or is this also done? 🤔

https://go.libelektra.org/src/bindings/go-elektra should return Note: the https:// isn't needed and without it go also tries ssh if https fails.

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta
      name="go-import"
      content="go.libelektra.org git github.com/ElektraInitiative/libelektra"
    />
  </head>
  <body></body>
</html>

markus2330 commented 1 year ago

Changed it.

tmakar commented 1 year ago

Changed it.

@markus2330 @kodebach

Ok, so unfortunately this again does not work and go get fails. I'm afraid that we can not use go.libelektra.org in combination with a go module that is not at the root of the repository. It seems that go assumes that go-import contains the repository path where also the go.mod file is located.

So, my suggestion would be the following:

I change this PR to also include the new module name (github.com/ElektraInitiative/libelektra/src/bindings/go-elektra)
Then we merge this
Then we merge the fix for the release (0.10.0), tag will be v0.10.0 (https://github.com/ElektraInitiative/libelektra/pull/4933)
Then start the release pipeline again

After step 4 external go programs should be able to use the go get github.com/ElektraInitiative/libelektra/src/bindings/go-elektra with version tag v0.10.0 also and not just commit hash.

And just a quick note, I think its very good that we have our go binding included into our build system.

tmakar commented 1 year ago

jenkins build libelektra please

tmakar commented 1 year ago

jenkins build libelektra please

tmakar commented 1 year ago

jenkins build libelektra please

tmakar commented 1 year ago

jenkins build libelektra please

kodebach commented 1 year ago

So, my suggestion would be the following:

I agree that using github.com/... would be much simpler and is definitely good enough for the first release. But I still think go.libelektra.org can be made to work.

Some new findings (if we want to switch back to go.libelektra.org after the release):

It seems HTTP 403 for go.libelektra.org is a problem after all. Maybe we should return the same file for all of go.libelektra.org, go.libelektra.org/src, go.libelektra.org/src/bindings and go.libelektra.org/src/bindings/go-elektra. It seems go tries all of them and expects a successful response for all. (This is based on a go get -x with GOPROXY=direct).
The proxy.golang.org server also seems to have problems with the 403. If you open https://proxy.golang.org/go.libelektra.org/src/bindings/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info you'll see an error about it.

tmakar commented 1 year ago

So, my suggestion would be the following:

I agree that using github.com/... would be much simpler and is definitely good enough for the first release. But I still think go.libelektra.org can be made to work.

Some new findings (if we want to switch back to go.libelektra.org after the release):

It seems HTTP 403 for go.libelektra.org is a problem after all. Maybe we should return the same file for all of go.libelektra.org, go.libelektra.org/src, go.libelektra.org/src/bindings and go.libelektra.org/src/bindings/go-elektra. It seems go tries all of them and expects a successful response for all. (This is based on a go get -x with GOPROXY=direct).

The proxy.golang.org server also seems to have problems with the 403. If you open https://proxy.golang.org/go.libelektra.org/src/bindings/@v/a59fc7986097c11104c89c822ca5d650968ce83e.info you'll see an error about it.

Thanks again for your findings as well. @markus2330 if you also agree with having github.com/… instead of go.libelektra.org we can merge and do the release. Note, we need to merge https://github.com/ElektraInitiative/libelektra/pull/4933 after merging this, before doing the release.

markus2330 commented 1 year ago

Yes, I agree we will simply hard-code github.com. If we switch from github.com we need to reinvestigate.

ElektraInitiative / libelektra