hooks icon indicating copy to clipboard operation
hooks copied to clipboard

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

Open braindevices opened this issue 5 years ago • 19 comments
trafficstars

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)

  • Operating System+version: 10.15.4
  • Compiler+version: N/A
  • Conan version: 1.25.2
  • Python version: 3.8

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)

braindevices avatar Jun 01 '20 13:06 braindevices

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?

memsharded avatar Jun 01 '20 14:06 memsharded

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

braindevices avatar Jun 01 '20 15:06 braindevices

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.

memsharded avatar Jun 01 '20 16:06 memsharded

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

braindevices avatar Jun 01 '20 16:06 braindevices

according to conan get this value is not set

maddanio avatar Jun 01 '20 17:06 maddanio

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

maddanio avatar Jun 01 '20 17:06 maddanio

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 avatar Jun 01 '20 17:06 memsharded

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?

memsharded avatar Jun 01 '20 18:06 memsharded

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

braindevices avatar Jun 01 '20 21:06 braindevices

@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!

memsharded avatar Jun 01 '20 23:06 memsharded

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?

czoido avatar Jun 02 '20 05:06 czoido

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.

maddanio avatar Jun 02 '20 07:06 maddanio

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.

jgsogo avatar Jun 02 '20 08:06 jgsogo

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.

memsharded avatar Jun 02 '20 08:06 memsharded

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

maddanio avatar Jun 02 '20 08:06 maddanio

Or, yes, let unversioned data through

maddanio avatar Jun 02 '20 08:06 maddanio

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.

jgsogo avatar Jun 02 '20 08:06 jgsogo

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

maddanio avatar Jun 02 '20 11:06 maddanio

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.

jgsogo avatar Jun 02 '20 12:06 jgsogo