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

Integration repository failing to install from HEAD apparently due to version comparison bug in HACS #4423

Open
8 tasks done
DanielBaulig opened this issue Feb 6, 2025 · 13 comments
Labels
issue:backend For issues with the backend/integration

Comments

@DanielBaulig
Copy link

System Health details

N/A

Checklist

  • I'm running the newest version of HACS https://github.com/hacs/integration/releases/latest
  • I have enabled debug logging for my installation.
  • I have filled out the issue template to the best of my ability.
  • I have read https://hacs.xyz/docs/help/issues/
  • This issue is related to the backend (integration part) of HACS.
  • This issue only contains 1 issue (if you have multiple issues, open one issue for each issue).
  • This is a bug and not a feature request.
  • This issue is not a duplicate issue of currently open or issues pending release.

Describe the issue

Disclaimer: I am reporting this issue on behalf of some of my integrations users. It is entirely based on reading the HACS code involved and is not a result of me running or using HACS. Some items on the checklist do not seem applicable in this case.

I had some users of my small integration report that they are experiencing issues updating / installing my integration via HACS. I never used HACS myself and didn't do anything to support HACS specifically with my integration or made any changes to the integration in the last year, but decided to take a look.

The error message my users are getting ("The version fe0e49abe0f532de31f532d077ffd968607b75a8 for this integration can not be used with HACS") seems to be originating from here:

if ref == self.data.last_version:
target_manifest = self.repository_manifest
else:
target_manifest = await self.get_hacs_json(version=ref)
if target_manifest is None:
raise HacsException(
f"The version {ref} for this {
self.data.category.value} can not be used with HACS."
)

Since I do not have a hacs.json in my repository, it appears like this would happen if self.data.version is not equal to ref. It appears like self.data.version is being pulled from manifest.json in my integration directory. I have defined the version as "0.2.0" in my manifest.json. However, from the error messages reported I can see that the value of ref seems to be a SHA hash, presumably the HEAD commit hash of my repository. Now that is not the same as "0.2.0", which would explain the error message and failure to install/update.

I blamed some of the code paths involved to see if there were any recent changes in HACS that may have caused this (I got two reports of this very recently and never before), but couldn't find anything.

I believe unless one is willing to go through a significant amount of proof of work to generate a collision (like some millions of years of hash time), the version string in manifest.json can never be the commit hash of the commit that contains it.

If the repository being installed uses the "If you don't publish releases in your repository, HACS will use the files in the branch marked as default." case as described here HACS probably should not compare the commit hash value provided via ref to the version string specified in the mainfest.json. Or I guess not support this case anymore.

Thoughts?

Side note: I am going to try to use a release to resolve this issue for me and my integration personally, but the "HACS will use the files in the branch marked as default" case seems to currently be broken.

Reproduction steps

See above.

Debug logs

N/A

Diagnostics dump

N/A

@DanielBaulig DanielBaulig added the issue:backend For issues with the backend/integration label Feb 6, 2025
@hacs-bot
Copy link

hacs-bot bot commented Feb 6, 2025

Make sure you have read the issue guidelines and that you filled out the entire template.

If you have an issue identical to this, do not add comments like "same here", "i have this too", instead add a 👍 reaction to the issue description. Thanks! 👍

@DanielBaulig DanielBaulig changed the title Integration repository apparently causing versioning bug in HACS Integration repository failing to install apparently due to version bug in HACS Feb 6, 2025
@DanielBaulig DanielBaulig changed the title Integration repository failing to install apparently due to version bug in HACS Integration repository failing to install from HEAD apparently due to version bug in HACS Feb 6, 2025
@DanielBaulig DanielBaulig changed the title Integration repository failing to install from HEAD apparently due to version bug in HACS Integration repository failing to install from HEAD apparently due to version comparison bug in HACS Feb 6, 2025
@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

hacs.json is required.

@DanielBaulig
Copy link
Author

@ludeeus Thank you for the response.

This requirement is not documented here: https://www.hacs.xyz/docs/publish/integration/#requirements
And at least the code in _ensure_download_capabilities does not appear to enforce this requirement either.

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

It is not there because its not spesific to integrations.
Its here https://www.hacs.xyz/docs/publish/start/#hacsjson

@DanielBaulig
Copy link
Author

Thank you! I did not publish my integration to HACS though. I believe some of the users are adding my repository via their HACS installation without me having published my repository to HACS. Is that possible?

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

That is possible https://hacs.xyz/docs/faq/custom_repositories/

@DanielBaulig
Copy link
Author

Thanks for answering all my questions :)
Is it expected for these custom repositories to still include a hacs.json, too?

If so, it may make sense to explicitly check for the presence of hacs.json in _ensure_download_capabilities and maybe leave a comment with that code that directs to developer documentation for how to make a repository compatible with HACS. Would have helped here and saved us both a bunch of time.

Feel free to close if the expectation is for custom repositories to have a hacs.json. Thanks!

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

Every repository that are to be used with hacs needs to have that file.

It maybe have worked prior to v2, but to that version a lot of issues were fixed by making it more strict.

This can not be checked when you add the repository, in case you need to download a pre-release or a older tag or anything.

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

As for checking for hacs.json in _ensure_download_capabilitie, im not sure what you mean, as that is the only thing it does?
It checks if that file exist, and the contents of it?

@DanielBaulig
Copy link
Author

DanielBaulig commented Feb 6, 2025

@ludeeus I mean this here:

async def _ensure_download_capabilities(self, ref: str | None, **kwargs: Any) -> None:
"""Ensure that the download can be handled."""
target_manifest: HacsManifest | None = None
if ref is None:
if not self.can_download:
raise HacsException(
f"This {
self.data.category.value} is not available for download."
)
return
if ref == self.data.last_version:
target_manifest = self.repository_manifest
else:
target_manifest = await self.get_hacs_json(version=ref)
if target_manifest is None:
raise HacsException(
f"The version {ref} for this {
self.data.category.value} can not be used with HACS."
)
if (
target_manifest.homeassistant is not None
and self.hacs.core.ha_version < target_manifest.homeassistant
):
raise HacsException(
f"This version requires Home Assistant {
target_manifest.homeassistant} or newer."
)
if target_manifest.hacs is not None and self.hacs.version < target_manifest.hacs:
raise HacsException(f"This version requires HACS {
target_manifest.hacs} or newer.")

It appears to verify if the repository and the specific version of it (i.e. commit / tag / whatever) is installable using HACS. It implicitly uses hacs.json, but there is a happy path that is completely agnostic towards if the repository contains a hacs.json or not (specifically if the repository contains tagged releases on Github). By adding a tagged release to my repository my users were able to install it via HACS, even without adding a hacs.json.

The code in _ensure_download_capabilitie appears to try to work without the presence of a hacs.json and technically does not enforce the presence of a hacs.json - which may make sense if the desire here is to support the installation of custom repositories without explicitly requiring them to add support for HACS.

But in that case the repository must have tagged releases, because otherwise the comparison between commit hash ref and self.data.latest_version will always fail and branch into the hacs.json path (i.e. target_manifest = await self.get_hacs_json(version=ref)).

So I think one of two paths are ideal (not withstanding technical limitations):

  1. Be more restrictive and explicit by checking for the presence of hacs.json in the specific version/tag/commit of the repository in _ensure_download_capabilities (e.g. by checking await self.get_hacs_json(version=ref) earlier and always)
  2. Be less restrictive and do not explicitly require hacs.json for custom repositories, but allow them to be installed from HEAD by fixing the ref hash to self.data.latest_version comparison.

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

Meh… there is a bug there.
It does what i think it does, as self.repository_manifest is also hacs.json, but this one defaults to {} for releases.

I'm not actually sure i want to fix that one, at least not untill V3.

@DanielBaulig
Copy link
Author

For releases it uses target_manifest = self.repository_manifest -- i.e. the Home Assistant manifest.

But on that the presence of hacs.json is not enforced. It is weird that if a user installs from HEAD instead of a release it suddenly is required. Especially since we are talking about custom repositories. The authors of those repositories may have never intended on their repositories to be installed via HACS or may not even know about HACS.

I think it would be better if the behavior was consistent between installing from releases or from HEAD. I think it might make sense to have both not require presence of hacs.json for custom repositories (that seems to be the whole purpose of custom repositories, to install integrations who's authors for whatever reason do not want to or can explicitly support HACS), but that's your call in the end.

But I agree, making it more restrictive may have to wait until a major version release. That will definitely break people that are installing repositories without hacs.json via releases.

@ludeeus
Copy link
Member

ludeeus commented Feb 6, 2025

For releases it uses target_manifest = self.repository_manifest -- i.e. the Home Assistant manifest.

No, hacs.json is the repository manifest.
https://github.com/hacs/integration/blob/main/custom_components/hacs/repositories/base.py#L314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue:backend For issues with the backend/integration
Projects
None yet
Development

No branches or pull requests

2 participants