conan
conan copied to clipboard
conan-io
[bug] Suggested replacement of SCM attribute with export/source is broken
Environment details
- Conan version: 1.62.0
Steps to reproduce
According to the Conan Migration Guide the SCM attribute should be replaced with export/source methods as follows:
def export(self):
git = Git(self, self.recipe_folder)
scm_url, scm_commit = git.get_url_and_commit()
update_conandata(self, {"sources": {"commit": scm_commit, "url": scm_url}})
def source(self):
git = Git(self)
sources = self.conan_data["sources"]
git.clone(url=sources["url"], target=".")
git.checkout(commit=sources["commit"])
However, on Conan 1.62.0 this is obviously broken. In an in-cache build Conan seems to copy conandata.yml into the source folder before calling source(). This will make clone() fail because the directory is not empty. Relocating the source folder by setting self.folders.source in the layout() method works for in-cache builds but not in the local build workflow, because there we need to match the layout imposed by the repository structure.
This is a blocker for our migration efforts. I haven't been able to find a workaround and I cannot continue work until I have a solution.
Hi @jasal82
Thanks for your report.
It would be good to have a fully reproducible case. We have this tested in Conan 1.X in our suite in https://github.com/conan-io/conan/blob/2985accf41e9380ba5b12dd8b4392b904abb4c21/conans/test/functional/tools/scm/test_git.py#L316, and seems to behave well
I thought that a pre-existing conandata.yml could be an issue, but I added it to the test, and still seems to be working fine, this is the full modified test for reference:
class TestGitBasicSCMFlowExistingConanData:
""" Build the full new SCM approach:
- export() captures the URL and commit with get_url_and_commit(
- export() stores it in conandata.yml
- source() recovers the info from conandata.yml and clones it
"""
conanfile = textwrap.dedent("""
import os
from conan import ConanFile
from conan.tools.scm import Git
from conan.tools.files import load, update_conandata
class Pkg(ConanFile):
name = "pkg"
version = "0.1"
def export(self):
git = Git(self, self.recipe_folder)
scm_url, scm_commit = git.get_url_and_commit()
update_conandata(self, {"sources": {"commit": scm_commit, "url": scm_url}})
def layout(self):
self.folders.source = "."
def source(self):
git = Git(self)
sources = self.conan_data["sources"]
git.clone(url=sources["url"], target=".")
git.checkout(commit=sources["commit"])
self.output.info("MYCMAKE: {}".format(load(self, "CMakeLists.txt")))
self.output.info("MYFILE: {}".format(load(self, "src/myfile.h")))
self.output.info(f"MYVALUE: {self.conan_data['key']}!!")
def build(self):
cmake = os.path.join(self.source_folder, "CMakeLists.txt")
file_h = os.path.join(self.source_folder, "src/myfile.h")
self.output.info("MYCMAKE-BUILD: {}".format(load(self, cmake)))
self.output.info("MYFILE-BUILD: {}".format(load(self, file_h)))
self.output.info(f"MYVALUE-BUILD: {self.conan_data['key']}!!")
""")
def test_full_scm(self):
folder = os.path.join(temp_folder(), "myrepo")
url, commit = create_local_git_repo(files={"conanfile.py": self.conanfile,
"src/myfile.h": "myheader!",
"CMakeLists.txt": "mycmake",
"conandata.yml": "key: value"}, folder=folder)
c = TestClient(default_server_user=True)
c.run_command('git clone "{}" .'.format(url))
c.run("create .")
assert "pkg/0.1: MYCMAKE: mycmake" in c.out
assert "pkg/0.1: MYFILE: myheader!" in c.out
assert "pkg/0.1: MYVALUE: value!!" in c.out
c.run("upload * --all -c")
# Do a change and commit, this commit will not be used by package
save_files(path=folder, files={"src/myfile.h": "my2header2!"})
git_add_changes_commit(folder=folder)
# use another fresh client
c2 = TestClient(servers=c.servers)
c2.run("install pkg/0.1@ --build=pkg")
assert "pkg/0.1: MYCMAKE: mycmake" in c2.out
assert "pkg/0.1: MYFILE: myheader!" in c2.out
assert "pkg/0.1: MYVALUE: value!!" in c2.out
# local flow
c.run("install .")
c.run("build .")
assert "conanfile.py (pkg/0.1): MYCMAKE-BUILD: mycmake" in c.out
assert "conanfile.py (pkg/0.1): MYFILE-BUILD: myheader!" in c.out
assert "conanfile.py (pkg/0.1): MYVALUE-BUILD: value!!" in c.out
def test_branch_flow(self):
""" Testing that when a user creates a branch, and pushes a commit,
the package can still be built from sources, and get_url_and_commit() captures the
remote URL and not the local
"""
url = git_create_bare_repo()
c = TestClient(default_server_user=True)
c.run_command('git clone "{}" .'.format(url))
c.save({"conanfile.py": self.conanfile,
"src/myfile.h": "myheader!",
"CMakeLists.txt": "mycmake",
"conandata.yml": "key: value"})
c.run_command("git checkout -b mybranch")
git_add_changes_commit(folder=c.current_folder)
c.run_command("git push --set-upstream origin mybranch")
c.run("create .")
assert "pkg/0.1: MYCMAKE: mycmake" in c.out
assert "pkg/0.1: MYFILE: myheader!" in c.out
assert "pkg/0.1: MYVALUE: value!!" in c.out
c.run("upload * --all -c")
rmdir(c.current_folder) # Remove current folder to make sure things are not used from here
# use another fresh client
c2 = TestClient(servers=c.servers)
c2.run("install pkg/0.1@ --build=pkg")
assert "pkg/0.1: MYCMAKE: mycmake" in c2.out
assert "pkg/0.1: MYFILE: myheader!" in c2.out
assert "pkg/0.1: MYVALUE: value!!" in c2.out
@memsharded I created a reproducible test case here https://github.com/jasal82/test. When running conan create . it immediately fails with
ERROR: mytest/0.1: Error in source() method, line 19
git.clone(url=sources["url"], target=".")
CalledProcessErrorWithStderr: Command 'git clone "/home/asaljo/code/bug/test" .' returned non-zero exit status 128.
fatal: destination path '.' already exists and is not an empty directory.
Hi @jasal82
This is expected, but sorry this wasn't more clearly documented.
You are missing the layout() method in your recipe. The layout() method is necessary to correctly enable the new feature, otherwise the conandata.yml is incorrectly copied to the source folder. As many other things in Conan 1.X, this was necessary for providing a non-breaking behavior for existing users.
Please add the layout() method to your recipe, and it will not fail.
This is the test, just in case:
def test(self):
conanfile = textwrap.dedent("""\
import os
from conan import ConanFile
from conan.tools.cmake import CMake
from conan.tools.files import update_conandata
from conan.tools.scm import Git
class Test(ConanFile):
name = "mytest"
version = "0.1"
generators = "CMakeToolchain"
def export(self):
git = Git(self, self.recipe_folder)
scm_url, scm_commit = git.get_url_and_commit()
update_conandata(self, {"sources": {"commit": scm_commit, "url": scm_url}})
def layout(self):
self.folders.source = "."
def source(self):
self.output.info(f"CWD: {os.getcwd()}")
self.output.info(f"SOURCE: {self.source_folder}")
self.output.info(f"FILES: {os.listdir(self.source_folder)}")
git = Git(self)
sources = self.conan_data["sources"]
git.clone(url=sources["url"], target=".")
git.checkout(commit=sources["commit"])
""")
folder = os.path.join(temp_folder(), "myrepo")
url, commit = create_local_git_repo(files={"conanfile.py": conanfile,
"src/main.cpp": ""}, folder=folder)
c = TestClient(default_server_user=True)
c.run_command('git clone "{}" .'.format(url))
c.run("create .")
print(c.out)
@memsharded I see. This should definitely be documented in the migration guide directly in the SCM attribute section. I already found that setting self.folders.source fixes the problem but didn't know that you could set it to "." as well.
Once I define the layout() method, everything else seems to change behavior as well. For example, the generated files end up next to the conanfile which makes conan build fail because it cannot find the CMakePresets.json. I tried different strategies for setting self.folders.generators as well but I seem to be unable to replicate the previous 1.x behavior where the generated files were created in the install folder. How do I have to set it to get the "legacy" behavior, because I don't want to touch the existing Conan 1.x pipelines if I can avoid it. Our pipeline basically works as follows:
conan source -if installfolder -sf checkoutfolder checkoutfolderconan install -if installfolder checkoutfolder fullref/1.0.0@user/channelconan build --build-folder installfolder checkoutfolder
Setting self.folders.generators = "build" seems to fix it for the pipelines since they always choose installfolder as $(cwd)/build. But our developers usually do conan build .. directly from their build folder (which is not necessarily called build) or conan build --build-folder with a different build folder argument than build, so I'd like to set the generators directory to the value of that --build-folder argument with a fallback to the current working directory...
I am afraid that is not possible. The opt-in into the new Conan 2.0-ready behavior is done by the definition of the layout() method. Once the layout() method is defined, the --source-folder and --build-folder arguments should be dropped, the information is encoded in the layout(), and only the top --output-folder can be used (and in most cases it shouldn't be necessary either).
I'm sorry to say this, but the whole folder handling in Conan 1 vs 2 is an absolute mess. I can't find a way to guarantee a smooth migration on our end. How am I supposed to get our recipe maintainers to migrate their recipes to Conan 2 if it's not possible to do so without side effects? The pipelines would need to know if recipes implement a layout method or not in order to react to the changed filesystem layouts in the local build workflow. I cannot go without a layout() method because I need to replace the scm attribute with export()/source() which in turn forces me to set self.folders.source = "." which can only be done in the layout() method.
I'm sorry to say this, but the whole folder handling in Conan 1 vs 2 is an absolute mess.
But this is not Conan 1 to Conan 2, but about the change to use layout() in the same Conan 1.X scope, isn't it? If it was working in Conan 1 with layout() it should work the same in Conan 2. Just to make sure I understand the problems.
How am I supposed to get our recipe maintainers to migrate their recipes to Conan 2 if it's not possible to do so without side effects? The pipelines would need to know if recipes implement a layout method or not in order to react to the changed filesystem layouts in the local build workflow.
It is true that for the Conan 1->2 migration we had assumed that this was mostly done by now, as layout() method is now almost 3 years old. Also it seems that there would be two different use cases:
- the CI case, where
conan createshould keep working, no issue. - The developers case, where they can use
conan install + conan buildlocally.
Quick question: Are you using conan install + conan build also in CI? It would be good to understand the particular pains.
Final question, the issues would be that developers need to do 2 different CLI invocations, depending if the package has layout() or not, like:
conan install . --source-folder=xxx --build-folder=yyy+conan build --build-folder=yyyy(if notlayout())conan install . --output-folder=zzz+conan build --output-folder=zzz(iflayout())
Did I understand correctly the issue?
But this is not Conan 1 to Conan 2, but about the change to use
layout()in the same Conan 1.X scope, isn't it? If it was working in Conan 1 withlayout()it should work the same in Conan 2. Just to make sure I understand the problems.
Yes, that's correct. We don't use layout() yet in any of our recipes.
Quick question: Are you using
conan install + conan buildalso in CI? It would be good to understand the particular pains.
We must use local build workflow in the CI. That's because we want to run unit tests in separate build stages (cross-build!) and we need to extract the test suite executable from the build folder to pass it to the test stage. I tried porting this to a conan create workflow, but I gave up in the end. Deployers do not work because they seem to be unable to copy from the build folder and for manual copy I'd need to know the path to the build folder in the cache which can only be obtained reliably via conan cache path. But that requires a fully specified reference including recipe revision and package ID, which I don't get in machine-readable form from a conan create invocation. So we're back to local build workflow in CI.
Final question, the issues would be that developers need to do 2 different CLI invocations, depending if the package has
layout()or not, like:
conan install . --source-folder=xxx --build-folder=yyy+conan build --build-folder=yyyy(if notlayout())conan install . --output-folder=zzz+conan build --output-folder=zzz(iflayout())Did I understand correctly the issue?
It's not about the Conan 2 build. I'm trying to make the Conan 1 build work with Conan 2 compatible recipes. I think I found a way that works with or without layout() now, which would be
conan install . -if build -of build
conan build . -if build -bf build
I don't care about the internal structure of that build folder, because the relative location of test suite binary can be configured in the pipeline. However, I still need a way to detect if we need to run conan source before installing, because the pipeline is used for both 'recipe-committed-with-sources' and 'recipe-committed-separately' projects.
It's not about the Conan 2 build. I'm trying to make the Conan 1 build work with Conan 2 compatible recipes. I think I found a way that works with or without layout() now, which would be
Ok, that sounds great then
However, I still need a way to detect if we need to run conan source before installing, because the pipeline is used for both 'recipe-committed-with-sources' and 'recipe-committed-separately' projects.
Yes, I see, I think this would be a separate issue, and that would still be the same in Conan 2 I think, not really connected to the layout(). Maybe there is some other special characteristic in the name that you could leverage? For example one recommended flow when using recipes from conan-center-index is to use them without user/channel (so not necessary to change other requires in other ConanCenter recipes) and using your own recipes with user/channel. If the logic is there to disambiguate, then maybe that could be used?
conan install . -if build -of build conan build . -if build -bf build
@memsharded Unfortunately I can't get export-pkg working when a layout method (with cmake_layout) is present. I thought it should work if I do
conan export-pkg -if build -bf build .
but when executing package() Conan says it cannot find {root_dir}/build/Release (which should be {root_dir}/build/build/Release because the cmake layout subtree is created inside the manually specified build folder).
This works fine, even with a cmake_layout enabled:
mkdir build && cd build
conan install ..
conan build ..
conan export-pkg ..
What is the difference? I need to be able to run this same sequence of commands in Conan 1 without having to rely on the current working directory. That would fix all my pipeline problems.
I found the difference. This just worked by accident because I called the folder build... If I do this:
mkdir build2 && cd build2
conan install ..
conan build ..
then Conan will use build2 as the install folder and put the build output into build/.... I don't get the logic behind all of this. Why can I not just override the build folder specified in the layout with whatever is set in -bf?
I just skimmed through the Conan implementation. It seems that you completely removed the possibility to enforce a specific layout from command line once a layout() is defined in the conanfile. This breaks our concept of a generic pipeline with local build workflow. If every recipe can arbitrarily configure its folders, how should a pipeline be able to archive artifacts of intermediate steps or to optimize builds (by caching, etc). Previously we could just let the pipeline decide where things should be placed but now I have to deal with recipes which can do whatever they want and I don't have any option of enforcing a predictable layout, not even a common root directory for the output.
My suggestion: Add support for overriding layout from command line or make -bf work properly in export-pkg because it is clearly broken.