conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.11k stars 964 forks source link

Conan lockfile keeps changing #6119

Closed DavidZemon closed 4 years ago

DavidZemon commented 4 years ago

Every time I run conan graph lock ..., the lockfile is re-written. The core content hasn't changed, but hashes are regenerated and sometimes the order of the nodes changes. This makes it an awful pain to store that lockfile in the VCS repository.

Am I doing something wrong? Is there a workflow change I need to make? An option that can be added to lock command? Is this a bug?

memsharded commented 4 years ago

Hi @DavidZemon

Yes, this is intended as a feature. If some package actually changes during some process (like --build=missing and some dependency is re-built from sources), the lockfile will store those changes, as it is the only way of keeping that information. If for some reason, the original lockfile needs to be maintained, then it is necessary to make a copy of it.

A different issue is that the information of the lockfile is not changing at all, but just the way it is stored (possibly the order of the nodes, which are unordered dict internally), and that is causing those differences, then we should try to improve that. I guess that doing some sort() internally in the serialization to file should be enough. Could you give some details of the differences you are seeing? Maybe pasting here a couple of those diffs? Thanks!

DavidZemon commented 4 years ago

Old file:

{
 "profile_host": "[settings]\narch=armv7hf\narch_build=x86_64\nbuild_type=Release\ncompiler=gcc\ncompiler.libcxx=libstdc++11\ncompiler.version=7\nos=Linux\nos_build=Linux\nplatform=sitara\n[options]\n[build_requires]\n[env]\n",
 "graph_lock": {
  "nodes": {
   "12962a47-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19:01de722bad232946f9b792f33249c157fb84c24c",
    "options": "install_source=True"
   },
   "12962a48-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "wsbu-docgen/0.1.3@wsbu/stable#d85759233340fc32cc4127c5afeba471:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   },
   "12962a49-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01:f576e5b9d63ca3a96f350e00218d1fba5d292dd4",
    "options": "shared=True\ntest=True"
   },
   "12962a4a-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   },
   "12962a4b-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "Catch2/2.6.1@catchorg/stable#0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   },
   "12962a4c-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "googletest/1.8.1@wsbu/stable#76b5d3f2d6d39ba4fb4e2dbb7952c1af:09a5aca59994424cb00fe5de5f037b562a308fef",
    "options": "run_tests=False\nshared=False"
   },
   "12962a4e-0d3f-11ea-ace0-9941403aca0b": {
    "pref": null,
    "options": "cJSON:shared=True\ncJSON:test=True\niodb:build_modsim=True\niodb:public_docs=False\niodb:shared=True\niodb:with_docs=True\nlinux:install_source=True",
    "requires": {
     "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01": "12962a49-0d3f-11ea-ace0-9941403aca0b",
     "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17": "12962a4a-0d3f-11ea-ace0-9941403aca0b",
     "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af": "12962a4d-0d3f-11ea-ace0-9941403aca0b",
     "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19": "12962a47-0d3f-11ea-ace0-9941403aca0b"
    },
    "path": "/home/david/reusable/Documents/Programming/RedLion/base/.cache/conan/s5t/conanfile.txt"
   },
   "12962a4d-0d3f-11ea-ace0-9941403aca0b": {
    "pref": "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af:fb7050f1c2ebbdccd6142c43cdd4ddcaa44891ca",
    "options": "build_modsim=True\npublic_docs=False\nshared=True\nwith_docs=True",
    "requires": {
     "Catch2/2.6.1@catchorg/stable": "12962a4b-0d3f-11ea-ace0-9941403aca0b",
     "googletest/1.8.1@wsbu/stable": "12962a4c-0d3f-11ea-ace0-9941403aca0b",
     "wsbu-docgen/0.1.3@wsbu/stable": "12962a48-0d3f-11ea-ace0-9941403aca0b"
    }
   }
  }
 },
 "version": "0.1"
}

New file

{
 "profile_host": "[settings]\narch=armv7hf\narch_build=x86_64\nbuild_type=Release\ncompiler=gcc\ncompiler.libcxx=libstdc++11\ncompiler.version=7\nos=Linux\nos_build=Linux\nplatform=sitara\n[options]\n[build_requires]\n[env]\n",
 "graph_lock": {
  "nodes": {
   "71e48849-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19:01de722bad232946f9b792f33249c157fb84c24c",
    "options": "install_source=True"
   },
   "71e4884d-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af:fb7050f1c2ebbdccd6142c43cdd4ddcaa44891ca",
    "options": "build_modsim=True\npublic_docs=False\nshared=True\nwith_docs=True",
    "requires": {
     "Catch2/2.6.1@catchorg/stable": "71e4884a-0fac-11ea-8943-a9b58d6911e8",
     "googletest/1.8.1@wsbu/stable": "71e4884b-0fac-11ea-8943-a9b58d6911e8",
     "wsbu-docgen/0.1.3@wsbu/stable": "71e4884c-0fac-11ea-8943-a9b58d6911e8"
    }
   },
   "71e4884e-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01:f576e5b9d63ca3a96f350e00218d1fba5d292dd4",
    "options": "shared=True\ntest=True"
   },
   "71e4884b-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "googletest/1.8.1@wsbu/stable#76b5d3f2d6d39ba4fb4e2dbb7952c1af:09a5aca59994424cb00fe5de5f037b562a308fef",
    "options": "run_tests=False\nshared=False"
   },
   "71e48850-0fac-11ea-8943-a9b58d6911e8": {
    "pref": null,
    "options": "cJSON:shared=True\ncJSON:test=True\niodb:build_modsim=True\niodb:public_docs=False\niodb:shared=True\niodb:with_docs=True\nlinux:install_source=True",
    "requires": {
     "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01": "71e4884e-0fac-11ea-8943-a9b58d6911e8",
     "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17": "71e4884f-0fac-11ea-8943-a9b58d6911e8",
     "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af": "71e4884d-0fac-11ea-8943-a9b58d6911e8",
     "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19": "71e48849-0fac-11ea-8943-a9b58d6911e8"
    },
    "path": "/home/david/reusable/Documents/Code/base/.cache/conan/s5t/conanfile.txt"
   },
   "71e4884f-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   },
   "71e4884a-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "Catch2/2.6.1@catchorg/stable#0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   },
   "71e4884c-0fac-11ea-8943-a9b58d6911e8": {
    "pref": "wsbu-docgen/0.1.3@wsbu/stable#d85759233340fc32cc4127c5afeba471:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
    "options": ""
   }
  }
 },
 "version": "0.1"
}

Diff

Index: targets/s5t/conan/conan.lock
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- targets/s5t/conan/conan.lock    (revision bb62e9952c9fc21e7a7a73a7bd0f896eb40893bc)
+++ targets/s5t/conan/conan.lock    (date 1574704390791)
@@ -2,49 +2,49 @@
  "profile_host": "[settings]\narch=armv7hf\narch_build=x86_64\nbuild_type=Release\ncompiler=gcc\ncompiler.libcxx=libstdc++11\ncompiler.version=7\nos=Linux\nos_build=Linux\nplatform=sitara\n[options]\n[build_requires]\n[env]\n",
  "graph_lock": {
   "nodes": {
-   "12962a47-0d3f-11ea-ace0-9941403aca0b": {
+   "71e48849-0fac-11ea-8943-a9b58d6911e8": {
     "pref": "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19:01de722bad232946f9b792f33249c157fb84c24c",
     "options": "install_source=True"
    },
-   "12962a48-0d3f-11ea-ace0-9941403aca0b": {
-    "pref": "wsbu-docgen/0.1.3@wsbu/stable#d85759233340fc32cc4127c5afeba471:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
-    "options": ""
+   "71e4884d-0fac-11ea-8943-a9b58d6911e8": {
+    "pref": "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af:fb7050f1c2ebbdccd6142c43cdd4ddcaa44891ca",
+    "options": "build_modsim=True\npublic_docs=False\nshared=True\nwith_docs=True",
+    "requires": {
+     "Catch2/2.6.1@catchorg/stable": "71e4884a-0fac-11ea-8943-a9b58d6911e8",
+     "googletest/1.8.1@wsbu/stable": "71e4884b-0fac-11ea-8943-a9b58d6911e8",
+     "wsbu-docgen/0.1.3@wsbu/stable": "71e4884c-0fac-11ea-8943-a9b58d6911e8"
+    }
    },
-   "12962a49-0d3f-11ea-ace0-9941403aca0b": {
+   "71e4884e-0fac-11ea-8943-a9b58d6911e8": {
     "pref": "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01:f576e5b9d63ca3a96f350e00218d1fba5d292dd4",
     "options": "shared=True\ntest=True"
    },
-   "12962a4a-0d3f-11ea-ace0-9941403aca0b": {
-    "pref": "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
-    "options": ""
-   },
-   "12962a4b-0d3f-11ea-ace0-9941403aca0b": {
-    "pref": "Catch2/2.6.1@catchorg/stable#0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
-    "options": ""
-   },
-   "12962a4c-0d3f-11ea-ace0-9941403aca0b": {
+   "71e4884b-0fac-11ea-8943-a9b58d6911e8": {
     "pref": "googletest/1.8.1@wsbu/stable#76b5d3f2d6d39ba4fb4e2dbb7952c1af:09a5aca59994424cb00fe5de5f037b562a308fef",
     "options": "run_tests=False\nshared=False"
    },
-   "12962a4e-0d3f-11ea-ace0-9941403aca0b": {
+   "71e48850-0fac-11ea-8943-a9b58d6911e8": {
     "pref": null,
     "options": "cJSON:shared=True\ncJSON:test=True\niodb:build_modsim=True\niodb:public_docs=False\niodb:shared=True\niodb:with_docs=True\nlinux:install_source=True",
     "requires": {
-     "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01": "12962a49-0d3f-11ea-ace0-9941403aca0b",
-     "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17": "12962a4a-0d3f-11ea-ace0-9941403aca0b",
-     "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af": "12962a4d-0d3f-11ea-ace0-9941403aca0b",
-     "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19": "12962a47-0d3f-11ea-ace0-9941403aca0b"
+     "cJSON/1.7.0@wsbu/stable#8015fa7ba346d318e29ffb0ef2ce5b01": "71e4884e-0fac-11ea-8943-a9b58d6911e8",
+     "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17": "71e4884f-0fac-11ea-8943-a9b58d6911e8",
+     "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af": "71e4884d-0fac-11ea-8943-a9b58d6911e8",
+     "linux/4.9.119@wsbu/stable#2df0cc52a6a94aae8049f01f133b1f19": "71e48849-0fac-11ea-8943-a9b58d6911e8"
     },
-    "path": "/home/david/reusable/Documents/Programming/RedLion/base/.cache/conan/s5t/conanfile.txt"
+    "path": "/home/david/reusable/Documents/Code/base/.cache/conan/s5t/conanfile.txt"
    },
-   "12962a4d-0d3f-11ea-ace0-9941403aca0b": {
-    "pref": "iodb/2.3.1@wsbu/stable#5481b8d7a83e2c08c0c507e6151499af:fb7050f1c2ebbdccd6142c43cdd4ddcaa44891ca",
-    "options": "build_modsim=True\npublic_docs=False\nshared=True\nwith_docs=True",
-    "requires": {
-     "Catch2/2.6.1@catchorg/stable": "12962a4b-0d3f-11ea-ace0-9941403aca0b",
-     "googletest/1.8.1@wsbu/stable": "12962a4c-0d3f-11ea-ace0-9941403aca0b",
-     "wsbu-docgen/0.1.3@wsbu/stable": "12962a48-0d3f-11ea-ace0-9941403aca0b"
-    }
+   "71e4884f-0fac-11ea-8943-a9b58d6911e8": {
+    "pref": "esp-idf/3.1.3004@wsbu/stable#bfeacbaf0da7613d75aa164ecfdd5b17:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
+    "options": ""
+   },
+   "71e4884a-0fac-11ea-8943-a9b58d6911e8": {
+    "pref": "Catch2/2.6.1@catchorg/stable#0:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
+    "options": ""
+   },
+   "71e4884c-0fac-11ea-8943-a9b58d6911e8": {
+    "pref": "wsbu-docgen/0.1.3@wsbu/stable#d85759233340fc32cc4127c5afeba471:5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9",
+    "options": ""
    }
   }
  },

This is trivial to reproduce:

david@jesko:~/tmp$ conan new Foo/1.0
File saved: conanfile.py
david@jesko:~/tmp$ conan graph lock .
Requirements
Packages

Generated lockfile
david@jesko:~/tmp$ mv conan.lock conan.lock.first
david@jesko:~/tmp$ conan graph lock .
Requirements
Packages

Generated lockfile
david@jesko:~/tmp$ diff conan.lock conan.lock.first
5c5
<    "4c499c53-0fad-11ea-8943-a9b58d6911e8": {
---
>    "46dabdfb-0fad-11ea-8943-a9b58d6911e8": {
david@jesko:~/tmp$ 
memsharded commented 4 years ago

Hi @DavidZemon

In your example it makes sense, as you are generating the lockfile twice. Nodes are identified by UUIDs, it is impossible by definition that they will be the same. There is nothing in a conan graph lock command that would guarantee that the resulting lockfile will be the same, as the dependencies, revisions, etc, could have changed.

I thought you were talking about doing install|create operations using a lockfile. Does it also happens in that case?

DavidZemon commented 4 years ago

No, if I run install, the original lockfile is not modified, as expected.

However, this behavior of using newly generated UUIDs for nodes in the graph is real problem. I expect to be able to commit this file to VCS and I expect it not to change if nothing else has changed. I expect to be able to run conan graph lock and then review the resulting diff and say "oh good, nothing has changed, I don't need to re-run all of my tests." I do not expect that my git history is going to be filled to high heaven if one of the steps in my CI server config is to run conan graph lock followed by a commit to ensure the full firmware image has the latest versions of everything.

memsharded commented 4 years ago

The problem is that nodes in the graph needs unique identifiers, as the package reference might actually correspond to more than one node in the graph. There is no easy way to make the node IDs to be the same between consecutive calls that doesn't share any data. Comparing 2 different lockfiles captured independently in 2 different times hasn't been considered as a use case in the design of lockfiles.

I understand the issue, and I think it would be a good thing to have, but honestly, I cannot think of any reasonable solution that would achieve this. I will try to think of something, but suggestions welcome.

DavidZemon commented 4 years ago

starting at leaf nodes with no dependencies, the text of the node could be hashed and used the node ID. go up from there. that would work, right?

For my purposes, I have no need to combine build order and lock file features in one. My idea of a lockfile is something that tells me all of the components in a build... no graph is needed, let alone UUIDs for nodes in that graph.

memsharded commented 4 years ago

starting at leaf nodes with no dependencies, the text of the node could be hashed and used the node ID. go up from there. that would work, right?

That is the point, no, that will not work. For example, you can have a "build_requires" to cmake_installer package. This package has no dependencies (leaf node). As "build_requires" are private, you get one private node for each node in the graph that needs to be built from sources. They are not the same node because they are private, they can in fact be different versions, or have different options even if it is the same version. So they require to have some distinct unique ID, so they can be addressed and differentiated from the others.

DavidZemon commented 4 years ago

I think I understand your situation, but I don't understand where the problem comes from. Here's how I interpreted your situation:

Package A (my full firmware image) depends on B and C. B has a build_requires on X and, in its configure() method, it sets X['with_foo'] = True. C also has build_requires on X, but set's X['with_foo'] = False.

If that is, indeed, the situation you're referring to, it should be fine. This means you expect your lockfile to have 5 nodes total: A, B, C, X:with_foo=True, X:with_foo=False. If you hash the strings for each of these nodes, starting with the two X's, you'll get unique hashes for each one of them.

Am I misunderstanding some part of this?

memsharded commented 4 years ago

The problem there is what defines the string to be hashed. The full state of a node? All variables, settings, options, dependencies? What happens to configuration erasure (like when data is provided as input to a recipe, but later is cleaned, because it doesn't affect the packageID, like self.cpp_info.header_only()? Then you should use the "input" configuration instead of the final configuration? But sometimes this configuration doesn't even apply, will introduce unnecessary variability too.

Even in the case that you have exactly the same state of a node, you still need a unique different ID for those, otherwise your graph model would be broken, as those nodes collapse into one, when they are really different nodes.

I think that using the state is not a possibility, I am thinking of ways of the graph generating incrementing integers in a deterministic way, but doesn't seem easy, especially if we want to cover in the future the case of locally modifying lockfiles.

memsharded commented 4 years ago

Contributing a POC here: https://github.com/conan-io/conan/pull/6139, trying to understand better the scope. I am sure it will be more complicated than that, but at least as a starting point.

memsharded commented 4 years ago

PR was merged, so this will be released in 1.21.

Thanks again for reporting this!