west.yaml: update hal_espressif after SDHC porting

[sylvioalves]

Update revision reference for PR #71165 as needed.

Fix #72196

[zephyrbot]

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_espressif	https://github.com/zephyrproject-rtos/hal_espressif/commit/e554857b07d54e2152c6f1a9b39da6d6c13fe7f7	https://github.com/zephyrproject-rtos/hal_espressif/commit/c49581173bab8625d1f08d5c41d5a186afc240b3 (zephyr)	zephyrproject-rtos/hal_espressif@e554857b..c4958117

Note: This message is automatically posted and updated by the Manifest GitHub Action.

[jvermillard]

How could the PR introducing the wrong revision get merged, though?

Is the root cause a merge on the Zephyr side or a git "push force" that removes the targetted commit on the Espressif side?

[tomi-font]

Is the root cause a merge on the Zephyr side or a git "push force" that removes the targetted commit on the Espressif side?

In fact it does seem like the issue is caused by a force push on the hal_espressif repo. Shouldn't be allowed, right?

[str4t0m]

In fact it does seem like the issue is caused by a force push on the hal_espressif repo. Shouldn't be allowed, right?

I believe the issue is that the Zephyr PR referenced the commit hash of the unmerged HAL PR and was merge before the HAL PR.

[tomi-font]

I believe the issue is that the Zephyr PR referenced the commit of the unmerged HAL PR and was merge before the HAL PR.

You are correct: https://github.com/zephyrproject-rtos/hal_espressif/pull/276/commits

[str4t0m]

I seems to not break CI yet, but should be tag it as trivialsuch that it it will be merged sooner?

[fabiobaltieri]

Merged, could have used a better commit message though, something describing what was incorrect with the previous hash.

[sylvioalves]

I believe the issue is that the Zephyr PR referenced the commit of the unmerged HAL PR and was merge before the HAL PR.

You are correct: https://github.com/zephyrproject-rtos/hal_espressif/pull/276/commits

Sorry for this. Indeed that hal PR should have been merged before its counterpart in Zephyr.

[tomi-font]

Sorry for this. Indeed that hal PR should have been merged before its counterpart in Zephyr.

Yeah, and the commit hash referenced should have been the one in the repo after the PR was merged instead of the one in the PR.

Could we have an automatic check to detect this before PRs are merged into main in the future ?

[fabiobaltieri]

Could we have an automatic check to detect this before PRs are merged into main in the future ?

That would be ideal but unfortunately the way modules are managed is not quite uniform enough to allow writing such a check in a reliable way (not that I can think of anyway), specifically the "main" branch of modules can have different names. Though we could check that the commit it's part of any branch in the repo. If you have cycles to give it a try the code is here: https://github.com/zephyrproject-rtos/action-manifest/blob/main/action.py

[aescolar]

@sylvioalves please:

1. In the HAL module repo create a branch or tag the commit to ensure it is not garbage collected, and name it something that makes it very evident that it should not be deleted. And protect that branch in github.
2. Send a hotfix PR to Zephyr to correct the west manifest to point to the merged commit sha. (this was the hotfix, sorry for the noise)

Without 1. we will end up quite soon with this commits in Zephyr being unfetchable by west again

[sylvioalves]

@aescolar We usually make sure to update west.yaml only after having it merged into default/protected hal_espressif repo. This was really a mistake that has never happened before and I will, of course, pay more attention to it. Thanks for the recommendation.