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.

Sign up for GitHub

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

Jump to bottom

configs/*: update Dasharo EDK2 revision to get DasharoPayloadPkg #559

Merged
merged 3 commits into from
Sep 13, 2024
Merged

configs/*: update Dasharo EDK2 revision to get DasharoPayloadPkg #559

merged 3 commits into from
Sep 13, 2024

Conversation

SergiiDmytruk
Copy link
Member

@SergiiDmytruk SergiiDmytruk commented Sep 10, 2024 •
edited
Loading

Commit 05adc35 pointed EDK2 revision at a commit that's not pulled by git clone (comes from a removed or force-pushed branch) resulting in a non-buildable EDK2 checkout.

Added detecting invalid commit hash on checkout (existing code detects only references, but not hashes).

Fixed configs/config.protectli_vp2410 which didn't checkout edk2-platforms due to the use of wrong repo URL.

See #519 (comment) and #519 (comment).

@SergiiDmytruk SergiiDmytruk mentioned this pull request Sep 10, 2024
Merged
krystian-hebel
krystian-hebel previously approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@krystian-hebel krystian-hebel left a comment

Choose a reason for hiding this comment

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

Approved, but let's wait for CI this time.

@miczyg1
Copy link
Contributor

miczyg1 commented Sep 10, 2024 •
edited
Loading

Commit 05adc35 pointed EDK2 revision at incompatible version (dasharo branch instead of rebased). Fixing this.

See #519 (comment) and #519 (comment).

That is not entirely correct. The parent of 433b7a3 is a0727d6, which belongs to rebased branch. So it was based on the rebased branch. Not sure though what was the real reason of failing...

@krystian-hebel
Copy link
Contributor

That is not entirely correct. The parent of 05adc35 is a0727d6, which belongs to rebased branch.

Parent of commit on coreboot repo is a commit on edk2 repo?

@miczyg1
Copy link
Contributor

miczyg1 commented Sep 10, 2024 •
edited
Loading

That is not entirely correct. The parent of 05adc35 is a0727d6, which belongs to rebased branch.

Parent of commit on coreboot repo is a commit on edk2 repo?

Oh no, of course not. I meant the parent of 433b7a3

@SergiiDmytruk
Copy link
Member Author

That is not entirely correct.

You're right, I update the commit.

Dasharo/edk2@433b7a3 has This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.. dasharo is checked out by default. git checkout silently failed to find the commit:

    Fetching new commits from https://github.com/Dasharo/edk2.git
    Checking out edk2 revision 433b7a33eeb5ead75546f595e494a2a56e2c6d71
fatal: reference is not a tree: 433b7a33eeb5ead75546f595e494a2a56e2c6d71

@SergiiDmytruk
configs/*: update Dasharo EDK2 revision to fix build
fa20469 
Commit 05adc35 pointed EDK2 revision at a commit that's not
pulled by `git clone` (comes from a removed or force-pushed branch)
resulting in a non-buildable EDK2 checkout.

Change-Id: Ie4eaa45667d3b1643bf734a9984f212d6dca505c
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
Copy link
Member Author

Also added || exit 1 to 2 git checkout commands so it's easier to see the error.

@krystian-hebel
Copy link
Contributor

Also added || exit 1 to 2 git checkout commands so it's easier to see the error.

This seems like something worth upstreaming.

krystian-hebel
krystian-hebel previously approved these changes Sep 10, 2024
View reviewed changes
@SergiiDmytruk
payloads/edk2/Makefile: detect invalid commit hash on checkout
9a7523f 
`git rev-parse reference` doesn't fail on unknown commit hash unless
something like `^{object}` is appended (`^{commit}` can probably be used
as well).

Change-Id: I7ef39aeee2e902ac2fad6ac41b546c47418e1dec
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
configs/config.protectli_vp2410: use Dasharo/edk2-platforms
313ef1f 
Change-Id: Ief5292d59af43b4f6ccda31c188a089ce3c6216f
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
Copy link
Member Author

The new check found an issue with one of the configs (313ef1f), hence multiple pushes. Sent modified fix to upstream (didn't notice existing check initially).

@SergiiDmytruk
Copy link
Member Author

Ping.

@krystian-hebel krystian-hebel merged commit 313ef1f into dasharo Sep 13, 2024
41 checks passed
@krystian-hebel krystian-hebel deleted the fix-edk2-commit-in-configs branch September 13, 2024 10:33
@krystian-hebel krystian-hebel mentioned this pull request Sep 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krystian-hebel krystian-hebel krystian-hebel approved these changes

@mkopec mkopec Awaiting requested review from mkopec

@miczyg1 miczyg1 Awaiting requested review from miczyg1

@philipandag philipandag Awaiting requested review from philipandag

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SergiiDmytruk @miczyg1 @krystian-hebel