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

Bundling not working since v0.10.0 #273

Closed
josiasbruderer opened this issue Nov 3, 2023 · 13 comments
Closed

Bundling not working since v0.10.0 #273

josiasbruderer opened this issue Nov 3, 2023 · 13 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@josiasbruderer
Copy link
Contributor

From v0.10.0 ( 6b09374) bundling as described in README.md doesn't work anymore.

  1. Folder "plugin" was renamed to "go-ds-s3-plugin" which causes following error:

go: module github.com/ipfs/go-ds-s3@f3c87949d2ecf746794795b4b366b3ac6a1e631e found (v0.10.0), but does not contain package github.com/ipfs/go-ds-s3/plugin

  1. Following the same instructions but using the correct url (github.com/ipfs/go-ds-s3/go-ds-s3-plugin) doesn't work either:

plugin/loader/preload.go:11:1: import "github.com/ipfs/go-ds-s3/go-ds-s3-plugin" is a program, not an importable package

This leads me to the question:
Is bundling still meant to be used? If yes: Is there a way to build kubo (bundled with go-ds-s3) without reverting changes of v0.10.0 in go-ds-s3-plugin/* (which would break building process of v0.10.0 anyway)?

Thanks for your support! :)

Information to reproduce:

  • go version go1.21.2
  • kubo version 0.23.0
  • go-ds-s3 version v0.10.0
export GO111MODULE=on
git clone https://github.com/ipfs/kubo
cd kubo
git checkout v0.23.0

go get github.com/ipfs/go-ds-s3/[email protected]

echo -en "\ns3ds github.com/ipfs/go-ds-s3/go-ds-s3-plugin 0" >> plugin/loader/preload_list

make build # failes as described
go mod tidy
make build # failes as described
@josiasbruderer josiasbruderer added the need/triage Needs initial labeling and prioritization label Nov 3, 2023
Copy link

welcome bot commented Nov 3, 2023

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment.
Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

  • "Priority" labels will show how urgent this is for the team.
  • "Status" labels will show if this is ready to be worked on, blocked, or in progress.
  • "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

@Jorropo
Copy link
Contributor

Jorropo commented Nov 6, 2023

Thx for the report, I was able to get it working doing:

export GO111MODULE=on
git clone https://github.com/ipfs/kubo
cd kubo
git checkout v0.23.0

echo -en "\ns3ds github.com/ipfs/go-ds-s3 0" >> plugin/loader/preload_list

make build # expected to fail
go mod tidy
make build # succeed

The readme needs to be updated.

@Jorropo
Copy link
Contributor

Jorropo commented Nov 6, 2023

This is incorrect, it succeed because I reverted the change I made to my preload_list accidently.

@hsanjuan broke the preload list in a2884d2.
I don't understand a2884d2's details but the old architecture where we used to have ./cmd/... package main and ./plugin package plugin is what should happen:

I have no time so I have copy-pasted what I have there (which includes Makefiles etc.). My stuff is a bit simpler and less verbose and does the same shrug.

I guess this could be reverted, and have to update the build system to proper way.

@hsanjuan
Copy link
Contributor

hsanjuan commented Nov 7, 2023

Yes, it is matter of having a main package separate from plugin code, or just renaming https://github.com/ipfs/go-ds-s3/blob/master/go-ds-s3-plugin/s3ds.go#L1 to not be main ?

@josiasbruderer can you try that, I mean just copying the file into Kubo.

Don't revert as the working auto-builds are more useful now that manually bundling, imho. @josiasbruderer why are you manually bundling when you can build it and drop it in the plugins/ folder?

@hsanjuan hsanjuan self-assigned this Nov 7, 2023
@hsanjuan hsanjuan added kind/bug A bug in existing code (including security flaws) and removed need/triage Needs initial labeling and prioritization labels Nov 7, 2023
@josiasbruderer
Copy link
Contributor Author

josiasbruderer commented Nov 7, 2023

Thanks @hsanjuan and @Jorropo for taking care of this!
Of course I can build the plugin and copy it to .ipfs/plugins but this doesn't help in my case:

To offer a kubo package with integrated s3 capabilities (for now on arch linux: kubo-s3-git) I decided to go with the bundling. This way I can 1. make sure that kubo and s3-plugin are compatible (considering the struggle with getting go version and all other stuff right), 2. proberly build and install using package manager.

I followed the idea of renaming renaming "main" back to "plugin" and modified all so that build process as it is still works but bundling is possible again. I therefore create a Pull-Request.

@softwareplumber
Copy link

This also caused me grief as when I updated the version in my Docker build (which I originally set up to use bundling for the exact reasons @josiasbruderer outlines above), it broke. I will switch my build to using a prebuilt binary for now but either the fix needs to go in or the discussion of bundling needs to be removed from the README :-)

@josiasbruderer
Copy link
Contributor Author

The PR was merged. Thanks @hsanjuan !

One problem persists:
"go get github.com/ipfs/go-ds-s3@latest" will clone v0.10.0 (f3c8794) instead of current master. I assume that a new tag has to be created.
Since I can't include tags in Pull-Requests, may I ask you @hsanjuan to update the version.json and create a corresponding tag?

@Jorropo
Copy link
Contributor

Jorropo commented Nov 23, 2023

@josiasbruderer you can create a pull request editing version.json yourself and it will make a tag once merged.

@hsanjuan
Copy link
Contributor

You can go get github.com/ipfs/go-ds-s3@master too.

@josiasbruderer
Copy link
Contributor Author

josiasbruderer commented Nov 23, 2023

Thanks for the information! I tried it using the commit hash (go get github.com/ipfs/go-ds-s3@01d7975) which cached the updated code (as ...@master will do). But Go will use latest when importing s3ds (f.E. in preload_list) anyway. In my understanding a new release is therefore unavoidable.

@josiasbruderer
Copy link
Contributor Author

josiasbruderer commented Nov 27, 2023

@Jorropo sorry to bother again... I updated version in PR #277 which was merged 4 days ago but no tag was created. The needed workflow was probably delete in PR #270.
I'm not sure if it is needed to get this workflow back or just creating the tag manually makes sense. Would you have a look at it again? Thanks!

@hsanjuan
Copy link
Contributor

Yes, I think I requested that because the Releases tab should include prebuilt binary releases and not be polluted with other tags and what not. I have added a v0.11.0 tag now.

@josiasbruderer
Copy link
Contributor Author

I tested bundling and everything seems to work again. Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

4 participants