conan-io / hooks

Official Conan client hooks
MIT License
33 stars 46 forks source link

[conan center] Fail for custom data in 'conandata.yml' (better than removing without noticing) #201

Open braindevices opened 4 years ago

braindevices commented 4 years ago

when I add custom data in conandata.yml.

system-packages:
  ubuntu:
    libunwind: libunwind-dev
    glib-2.0: libglib2.0-dev
  fedora:
    libunwind: libunwind-devel
    glib-2.0: glib-devel
  osx:
    libunwind: null
    glib-2.0: null
  fallback:
    libunwind: libunwind/1.3.1
    glib-2.0: glib/2.64.0

conan export works fine on ubuntu and fedora, but on osx, in the export folder, the conandata.yml end up this:

{}

Environment Details (include every applicable attribute)

Steps to reproduce (Include if Applicable)

create a conan package with conandata.yml containing some custom data fields. Then export. Check the conandata.yml in export folder.

Logs (Executed commands with output) (Include/Attach if Applicable)

memsharded commented 4 years ago

Seems a bug when using the scm functionality. Can you please verify that you are using exactly the same recipe, and using the same scm functionality in both cases?

braindevices commented 4 years ago

@memsharded There is NO scm involved. It has nothing to do with scm.

memsharded commented 4 years ago

And you don't have # scm_to_conandata # environment CONAN_SCM_TO_CONANDATA activated in your configuration? In either Linux or OSX? I suspect in osx might be activated.

braindevices commented 4 years ago

@memsharded I never changed the default value. is this by default activated?

maddanio commented 4 years ago

according to conan get this value is not set

maddanio commented 4 years ago

explicitly setting that to False in conan.config or setting the env variable to 0 did not help either.

memsharded commented 4 years ago

I have extended one of the existing tests covering the conandata.yml functionality in: https://github.com/conan-io/conan/pull/7129, to check that the values are maintained. Lets see what CI says.

memsharded commented 4 years ago

the https://github.com/conan-io/conan/pull/7129 is green, (it also runs in OSX). Do you have a full example, full code + commands to reproduce?

braindevices commented 4 years ago

@memsharded here is the example cause the problem on our osx.

memsharded commented 4 years ago

@SSE4 if you can run a quick test using https://github.com/braindevices/exportdata-hello in OSX, I have tried exactly that in Win/Linux and seems to work ok, and https://github.com/conan-io/conan/pull/7129 is implementing a test that pass on OSX. If you could please run it locally in your computer, this repo as-is, that would help. Thanks!

czoido commented 4 years ago

Hi @braindevices, I think this could be related to a hook used for conan-center-index because I ran the example in OSX and could not reproduce the issue. Are you running conan with conan-center-index hooks enabled by chance? Is the issue solved if you deactivate those hooks?

maddanio commented 4 years ago

interesting. why does that modify the conandata? I had that hook in there because it was recommended when developing recipes, and then I left it there.

jgsogo commented 4 years ago

Hi, @maddanio

We enforce in ConanCenter many restrictions that are not forced in Conan itself, and that could not match requirements for other users or companies.

Talking about conandata.yml file specifically: this is a file that evolves over time in ConanCenter because new versions are added. Many times a new version requires only to add the entry in the conandata.yml while everything is exactly the same. By trimming data from conandata.yml we ensure that the revision for old versions is not modified when a new entry for a different version is added to the yml file (same recipe, same exported conandata.yml file, so Conan computes the same hash for exported files) and we don't need to build binaries for existing versions.

memsharded commented 4 years ago

Ok, not a bug in Conan client then. Moving this to the hooks repo, in case we want to highlight this behavior, or try to maintain user data while trimming the versions only.

maddanio commented 4 years ago

I understand, but it would be nicer if Conan fails in that case, instead of changing the file...less head scratching :)

maddanio commented 4 years ago

Or, yes, let unversioned data through

jgsogo commented 4 years ago

As this is a hook for conan-center the way to go would be to fail/warn depending on the warning level. Currently (in Conan Center), we don't allow any other data to be in the conandata.yml file.

maddanio commented 4 years ago

Ok, it would be nice if it would just fail, is there any use case where it makes sense for it not to?

jgsogo commented 4 years ago

For Conan Center it isn't, for users that want to activate this hook for their own recipes and builds maybe they don't want to fail. But I agree that, if any of the checks included in the conan-center hook is useful outside, we can move that function to a dedicated hook.