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

xxhash: bump to v2.1.2 #15556

Closed

Conversation

geissonator
Copy link

@geissonator geissonator commented Mar 24, 2023

There is a known issue in v2.1.1:

cespare/xxhash#54

Fix that issue by bumping to the version with the fix.

This has been fixed in the main branch of etcd via the following:

f0f77fc

But it was a pretty major upgrade so just take the one piece we need for the etcd v3.5 release tag.

The OpenBMC project (https://github.com/openbmc) utilizes yocto/bitbake to build custom Linux images for embedded servers. I've been attempting to get an etcd bitbake recipe into yocto over in openembedded/meta-openembedded#616. When cross compiling etcd for an arm based BMC, we ran into this xxhash issue. Bitbake recipes tend to only point to tagged and released versions of code so that is why I'd like to get this fix backported into the 3.5 branch. For now, I've added this change as a patch to my etcd bitbake PR, but it's always preferred by the openembedded community that the fix be in the source project.

There is a known issue in v2.1.1:

  cespare/xxhash#54

Fix that issue by bumping to the version with the fix.

This has been fixed in upstream etcd via the following:

  etcd-io@f0f77fc

But it was a pretty major upgrade so just take the one piece we need for
the etcd v3.5 release tag.

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
@geissonator geissonator force-pushed the v3.5.7-update-xxhash branch from 51832a5 to c34f524 Compare March 24, 2023 16:24
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

Thanks for raising this @geissonator.

The code change itself looks ok, though it would be helpful if you could give a bit more context on why this should meet the bar to be backported to previous etcd releases, i.e. how does the -buildmode=plugin issue impact etcd specifically?

I had a quick look through some build code and I can't see us using that flag, I could easily be wrong though as still learning, perhaps downstream consumers are doing so.

@geissonator
Copy link
Author

geissonator commented Mar 24, 2023

The code change itself looks ok, though it would be helpful if you could give a bit more context on why this should meet the bar to be backported to previous etcd releases, i.e. how does the -buildmode=plugin issue impact etcd specifically?

Sure, let me add a few more details to the original message above.

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 24, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
@ahrtr
Copy link
Member

ahrtr commented Mar 24, 2023

Thanks @geissonator for the PR.

The concern is that github.com/cespare/xxhash/v2 is just a indirect dependency in all places. So I think we should bump the direct dependency (github.com/prometheus/client_golang in this example?) firstly, which depends on github.com/cespare/xxhash/v2, then execute ./scripts/fix.sh to automatically update all related indirect dependencies.

Please update this PR per suggestion above. But I am not sure it can pass the pipeline tests. Anyway, please try it out.

@geissonator
Copy link
Author

I did what you asked above, and noticed that in the end, my diff looks pretty much like if we just pulled in f0f77fc from the main branch of this repo. My goal with this PR was to just pull the bare minimum in to get xxhash updated and building in my scenario.

I have very minimal knowledge of go and it's modules. I also have very little understanding on where your 3.5 release branch is (i.e. only critical fixes?). If others have not reported this issue, then I'm ok if we just close out this PR so it's here for others if they run into the issue and I'll just carry the patch in our bitbake recipe and wait for 3.6.

I also could change this PR to pull in f0f77fc if you think that would be acceptable to backport to the 3.5 release.

@geissonator
Copy link
Author

Looks like we'll just wait for 3.6 and carry the patch in meta-openembedded.

geissonator added a commit to geissonator/meta-openembedded that referenced this pull request Apr 18, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Apr 18, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request May 2, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request May 2, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
geissonator added a commit to geissonator/meta-openembedded that referenced this pull request May 2, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request May 2, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request May 2, 2023
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Fixes:
| # github.com/cespare/xxhash/v2
| asm: xxhash_amd64.s:120: when dynamic linking, R15 is clobbered by a global
| variable access and is used here:
| 00092 (/home/pokybuild/yocto-worker/meta-oe/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s:120)	ADDQ	R15, AX
| asm: assembly failed

Upstream-Status: Backport [etcd-io/etcd@f0f77fc]

Limited PR with just this patch submitted via this PR:
  etcd-io/etcd#15556

Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants