Skip to content

Add FIXUP option, for automatic package hash.#40

Open
cmangla wants to merge 2 commits intoopenwrt:mainfrom
cmangla:pr-hash-fixup-option
Open

Add FIXUP option, for automatic package hash.#40
cmangla wants to merge 2 commits intoopenwrt:mainfrom
cmangla:pr-hash-fixup-option

Conversation

@cmangla
Copy link
Copy Markdown
Contributor

@cmangla cmangla commented Sep 3, 2024

Fixes #18
Fixes #35

@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Sep 3, 2024

It seems FIXUP=1 needs to be set when PKG_MIRROR_HASH:=skip is set. See:
https://openwrt.org/docs/guide-developer/packages#use_source_repository

@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Sep 3, 2024

@aparcar Please do "Squash and merge" if the PR is good and whenever it is convenient for you. Thanks!

@qwerttvv
Copy link
Copy Markdown

qwerttvv commented Sep 4, 2024

This PR worked perfect, THANKS!!!

@EkkoG
Copy link
Copy Markdown
Contributor

EkkoG commented Oct 29, 2024

I think when FIXUP=1, package should not compile, or there can have another option to enable fixup-only action

@aparcar
Copy link
Copy Markdown
Member

aparcar commented Oct 29, 2024

I don't understand the motivation for this PR. Could you please explain and add it to the commit message?

@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Oct 29, 2024

I don't understand the motivation for this PR. Could you please explain and add it to the commit message?

Let me run the motivation by you for your feedback, before I update the commits.

The motivation behind adding the FIXUP option is to generalize my specific motivation, which is to be able to compile a single package that sets PKG_MIRROR_HASH=skip. Others have expressed similar desires, such as in Issue #35 .

When PKG_MIRROR_HASH=skip and the FIXUP option is not specified in the OpenWRT build system for make check, a message is generated in the logs. In this GH Action, that message is filtered-in (see this line) as an error, and the build fails.

When the FIXUP option is specified, and in the Makefile we have PKG_MIRROR_HASH=skip, no log message regarding a hash check failure is generated, so with that this GH action is able to subsequently compile the package. This succeeds because the FIXUP option also fixes the Makefile to contain the correct value of PKG_MIRROR_HASH.

This PR adds the ability to specify the FIXUP option to make check (over here). I have also added a second, a more brute-force, option of IGNORE_PKG_HASH_ISSUE. This option simply disables the exit 1 statement when a package has a mismatched hash (over here).

I welcome suggestions for changes to this PR.

@qwerttvv
Copy link
Copy Markdown

When compiling individual packages, one may want to pass the FIXUP
option to `make check`. It can help with generating package hashes.
@cmangla cmangla force-pushed the pr-hash-fixup-option branch from df63054 to 684dfe0 Compare November 1, 2024 09:49
@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Nov 1, 2024

I don't understand the motivation for this PR. Could you please explain and add it to the commit message?

@aparcar Done. I kept only the FIXUP option in this PR for simplicity and clarity.

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 7, 2024

@cmangla changes looks O.K. If you have extra time I would also check the diff in the package Makefile and if FIXUP enable also suggest the correct hash. Totally optional but would be welcome

An alternative solution might be "--nocheck" to disable package check entirely.

@EkkoG
Copy link
Copy Markdown
Contributor

EkkoG commented Nov 7, 2024

@Ansuel I mostly use FIXUP to fix the hash in Makefile, not use it to avoid build failure. I think this is the right way use it, so I thinks when FIXUP=1, the compile should not happen.

@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Nov 7, 2024

@Ansuel I mostly use FIXUP to fix the hash in Makefile, not use it to avoid build failure. I think this is the right way use it, so I thinks when FIXUP=1, the compile should not happen.

@EkkoG Presumably, when you do use FIXUP to fix the hash in the Makefile, you do eventually compile it, right? In other words, the eventual goal of fixing the Makefile is to then launch a build with it. Given that, I do not see the point on insisting that the build should not happen when FIXUP=1.

Anyway, if the merge of this PR is predicated on this alternative mode of operation, please do let me know.

I do agree with @Ansuel though, that ideally the fixed Makefile should be presented so that the fixes can be retrieved and then the subsequent checks can pass without the FIXUP option.

@lars18th
Copy link
Copy Markdown

lars18th commented Mar 6, 2025

Any reason to not merge this enhancement?

@cmangla
Copy link
Copy Markdown
Contributor Author

cmangla commented Mar 6, 2025

Any reason to not merge this enhancement?

I suppose some people are of the opinion that this enhancement is not complete, in the sense that the internally generated corrected package hash is used to build the package, but that hash is not delivered to the end user. The end user may want to use the correct package hash to be able to build without the FIXUP option.

I am of the opinion that this PR could be merged as is, since it is useful on its own and currently in use by others, as is. And if there are people who really want the package hash to be delivered, either in the GH action logs or as an artifact, that could be done in a subsequent PR.

@lars18th
Copy link
Copy Markdown

lars18th commented Mar 7, 2025

I am of the opinion that this PR could be merged as is, since it is useful on its own and currently in use by others, as is. And if there are people who really want the package hash to be delivered, either in the GH action logs or as an artifact, that could be done in a subsequent PR.

I agree. However, perhaps you can include a parameter to generate optionally a text file with the hash. This could increase the likelihood of acceptance.

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

Successfully merging this pull request may close these issues.

"PKG_MIRROR_HASH:=skip" in Makefile, but build fails with "Package HASH check failed". Dot not fail when package hash configuration is skip

6 participants