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

The subhook upstream is gone #6398

Closed
dvzrv opened this issue Nov 3, 2024 · 20 comments
Closed

The subhook upstream is gone #6398

dvzrv opened this issue Nov 3, 2024 · 20 comments

Comments

@dvzrv
Copy link

dvzrv commented Nov 3, 2024

Hi! 👋

I'm trying to adapt the Arch Linux package setup for this project.

Unfortunately it appears that the upstream project subhook is now gone, but is still being used as a submodule:

https://github.com/tianocore/edk2/blob/47ba459fc1c2cda532b5d7cf16c9fdcbb200784d/.gitmodules#L28C1-L28C43

It would be great if you could fix this. Many thanks! 🙏

@os-d
Copy link
Contributor

os-d commented Nov 3, 2024

Thanks for the heads up. @mdkinney @spbrogan @makubacki , it indeed looks like subhook has been removed from GitHub and I don’t see where else it has migrated to. All of Zeex’s (the subhook owner) repos are gone. Looks like we will need to carry our own copy or choose a different path. I believe subhook was brought in for GoogleTest and that is the only current consumer.

@kuqin12
Copy link
Contributor

kuqin12 commented Nov 3, 2024

Is there any real usage for this submodule? I understand it is supported through the header. But the intended usage seems like a hack for big files that are hard to test. I understand this might be useful for existing codebase, but I think for newly developed modules, this model should not be encouraged.

In addition, the repo itself is incomplete and brought in issues of expanding to arm64 environment. Should we just drop its usage in edk2?

@mdkinney
Copy link
Member

mdkinney commented Nov 3, 2024

Yes. They are used from the macros that support using google mocks for internal functions. I will investigate what has happened and what alternatives may be available. The use case is for host based unit testing which is enabled for IA32/X64. Are there plans to expand host based unit testing to more architectures?

@kuqin12
Copy link
Contributor

kuqin12 commented Nov 3, 2024

I think expanding to more architectures will allow testing on the native environments (i.e. arm, risc-v). At this point, google test will only allow logic testing, which will be of limited usage if the routine is very low level like assembly functions.

@mdkinney
Copy link
Member

mdkinney commented Nov 3, 2024

Agree. Testing assembly function can be hard. If assembly code is in a lib function and it only uses instructions that are allowed from a user mode application, then host based unit testing is possible.

There are a number of updates that would be required in the UnitTestFrameworkPkg and potentially the stuart tool CI usages to
enable additional architectures for host based unit testing.

@makubacki
Copy link
Member

Considering how to improve/replace subhook usage is a productive conversation, but I think we should treat that as a separate topic with the attention/focus that it deserves. For now, many project builds are broken due to the submodule dependency and I think we should provide an alternative to support existing usage as the highest priority.

@mdkinney, do you think tianocore could host a repo with the code?

@mdkinney
Copy link
Member

mdkinney commented Nov 4, 2024

I will add mirror to TianoCore and will update status here and on mailing list

@mdkinney
Copy link
Member

mdkinney commented Nov 4, 2024

Mirror created: https://github.com/tianocore/edk2-subhook
PR opened to use mirror: #6402

@leiflindholm and @ajfish please review PR for use of edk2 mirror of subhook to unblock dev and CI clones.

@bssrikanth
Copy link

bssrikanth commented Nov 5, 2024

@mdkinney I see #6402 is now merged which will unblock CI runs on master branch. We have CIs which uses stable branch of edk2, they still fail :( would you consider tagging stable version soon?

@mdkinney
Copy link
Member

mdkinney commented Nov 5, 2024

There is a release planned for 11-22-24. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning

We can cherry-pick this fix to the previous stable tag edk2-stable202408 if that is required.

@bssrikanth
Copy link

There is a release planned for 11-22-24. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning

We can cherry-pick this fix to the previous stable tag edk2-stable202408 if that is required.

That would be helpful :)

@rnertney
Copy link

rnertney commented Nov 5, 2024

There is a release planned for 11-22-24. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning

We can cherry-pick this fix to the previous stable tag edk2-stable202408 if that is required.

This would be a much appreciated cherry pick.

@eric-unc
Copy link

eric-unc commented Nov 5, 2024

Yeah, I would also appreciate it :)

@mdkinney
Copy link
Member

mdkinney commented Nov 5, 2024

@lgao4 in order to complete this request, we need to follow the process outlined in RFC https://edk2.groups.io/g/rfc/message/474.

This would include

@mdkinney
Copy link
Member

mdkinney commented Nov 5, 2024

@gdong1
Copy link
Contributor

gdong1 commented Nov 6, 2024

Is it possible to fix the same build failure in the old stable branch (e.g.: edk2-stable202311)? The projects using the statable branch also failed to build. It would be great if this fix could apply to old stable branches.

@mdkinney
Copy link
Member

mdkinney commented Nov 6, 2024

Release published and edk2-stable202408.01 tag created:

https://github.com/tianocore/edk2/releases/tag/edk2-stable202408.01

@mdkinney
Copy link
Member

mdkinney commented Nov 6, 2024

@gdong1 That would be up to 5 hot fix releases and the process being followed https://edk2.groups.io/g/rfc/message/474 only documents supporting the most recent stable tag release.

@gdong1
Copy link
Contributor

gdong1 commented Nov 6, 2024

Got it. Thanks @mdkinney for this info.

adamcstephens pushed a commit to NixOS/nixpkgs that referenced this issue Nov 7, 2024
The URL of the subhook submodule has changed, as the original repository is no longer available.
tianocore/edk2#6398

fixes #353769
arnout pushed a commit to buildroot/buildroot that referenced this issue Nov 7, 2024
…edk2 download

All EDK2 releases <= edk2-stable202408 can't be fetched from git
anymore due to a missing git submodule as reported by [1].

Usually Buildroot fall-back using https://sources.buildroot.net thanks
to BR2_BACKUP_SITE where a backup of the generated archive is
available. But the BRConfigTest remove BR2_BACKUP_SITE default value
while generating the .config used by runtime tests. See [2].

Replace the BR2_BACKUP_SITE override from BRConfigTest in order to
continue testing EDK2 package using the usual backup site.

This workaround needs to be removed with the next EDK2 version bump
using this commit [3].

Fixes:
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258995977 (TestGrubX8664EFI)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258995962 (TestGrubi386EFI)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258995455 (TestFwts)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258995427 (TestIso9660Grub2Hybrid)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258995416 (TestIso9660Grub2EFI)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258992621 (TestEdk2)
https://gitlab.com/buildroot.org/buildroot/-/jobs/8258990697 (TestGrubAArch64EFI)

[1] tianocore/edk2#6398
[2] https://gitlab.com/buildroot.org/buildroot/-/commit/559bb33ae71dd2358ca2314ccb24b56cc3809fc1
[3] tianocore/edk2@95d8a1c

Signed-off-by: Romain Naour <romain.naour@smile.fr>
[Julien: insert link to [2] in commit log]
Signed-off-by: Julien Olivain <ju.o@free.fr>
@dvzrv
Copy link
Author

dvzrv commented Nov 10, 2024

Many thanks for looking into this!

I've successfully updated to edk2-stable202408.01 and I guess this can be closed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants