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

post_package hook corrupts packages #5588

Closed
memsharded opened this issue Aug 6, 2019 · 6 comments · Fixed by #5647
Closed

post_package hook corrupts packages #5588

memsharded opened this issue Aug 6, 2019 · 6 comments · Fixed by #5647

Comments

@memsharded
Copy link
Member

As it is done after the manifest is created, examples like the one in the docs:

 import os
 from conans import tools

 SIGNATURE = "this is my signature"

 def post_package(output, conanfile, conanfile_path, **kwargs):
     sign_path = os.path.join(conanfile.package_folder, ".sign")
     tools.save(sign_path, SIGNATURE)
     output.success("Package signed successfully")

Will break the manifest, and conan upload --check will reject the package as corrupted

@memsharded memsharded self-assigned this Aug 6, 2019
@memsharded memsharded added this to the 1.19 milestone Aug 6, 2019
@jgsogo
Copy link
Contributor

jgsogo commented Aug 8, 2019

This is something for the docs repo, right? Or is there anything to change/implement in Conan?

@memsharded
Copy link
Member Author

I think we should change the order of the hook, to allow file creating without breaking manifests.

@jgsogo
Copy link
Contributor

jgsogo commented Aug 9, 2019

I think it is useful to have a hook that is executed after the package is created and the manifest computed, so I would look for a different approach: the hook can tell Conan back if the package has been modified (return True/False value?), the hook itself can recompute the manifest (it would require exposing and documenting internal functions), or we can always add more hooks (I would prefer not to).

@memsharded
Copy link
Member Author

Yes, it might be useful, but right now I cannot think of any example that really needs to have the manifest already computed. We would need to have stronger evidence or users request to support this case.

Exposing internal functions to hooks doesn't seem the way to go, and having to rely on the user reminding to return True for correct behavior seems fragile.

In the meanwhile, the current approach for the use case that we already have example for (package signing) is broken, so the priority would be to fix it. If the package signing uses the manifest for its creation, it will be broken too when the manifest is recomputed, so wouldn't it be better if the manifest is not there?

@jgsogo
Copy link
Contributor

jgsogo commented Aug 9, 2019

We've been talking a lot in the past about generating quality artifacts after the package has been created (test reports, doxygen,...) and some users may have hooks that are using the packaged folder to generate those artifacts (store them somewhere else). I cannot know if having the manifest there already computed is important or not for those users.

I totally agree that there is not a good alternative, but changing this behavior may break someone. Some users will want to execute it before the manifest and others afterward (they might be using the manifest as a sentinel to know that the package is ready or even to mark a package as invalid an prevent the upload).

Let me know about the decision and I will write a PR for it.

@lasote
Copy link
Contributor

lasote commented Aug 19, 2019

Run it by default before the manifest calculation. If any issue or breaking behavior is reported we will think about it.

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

Successfully merging a pull request may close this issue.

3 participants