[bug] conan source does not properly handle the root folder

[timviola-cmi]

Describe the bug

When following the package development workflow

self.source_folder in the source() method is <conanfile_dir>/self.folders.source
however self.source_folder in all other methods is <conanfile_dir>/self.folders.root/self.folders.source

This makes it impossible to follow the package development workflow particularly when no_copy_source is true as both the source and build methods need to reference self.source_folder, but the path is different.

How to reproduce it

The issue is easily reproducible with the below simple conan file

from conan import ConanFile

class BasicConanfile(ConanFile):
    name = "conan_source_bug"
    version = "0.1"
    description = "Demonstrate a bug in conan source() method"
    no_copy_source = True
    
    def layout(self):
        self.folders.root = "root_folder"
        
    def source(self):
        self.output.info(f"In the source() method the Source folder is: {self.source_folder}")

    def build(self):
        self.output.info(f"In the build() method the Source folder is: {self.source_folder}")

Running conan source . and conan build . will yield the below results

[memsharded]

Thanks for your report @timviola-cmi

As clarified in https://github.com/conan-io/conan/pull/18377/files#r2112721523, the self.folders.root should point to a parent folder, not a child one. It would be necessary to clarify the problem that this self.folders.root = "root_folder" is trying to solve first.

[timviola-cmi]

I had put this message on the pull request conversation, but I think you wanted to make sure it was captured in the issue, so I'm replicating it here. We can continue the conversation in wherever is best.

The example from the documentation fails

When you run the example you can see that the source() method fails because it is looking for CMakeLists.txt in the conan directory. You can prove this by moving CMakeLists.txt into the conan directory and then conan source . will run successfully.

Conditions

I did see in the documentation that it said parent folder, but this fails even if following the parent folder guidance as mentioned above. The particular conditions under which this really matters:

1. The conanfile is not stored in the same repository as the source
2. You cannot use export_sources() because the source code should not be available to all package consumers
3. no_copy_source=True is used because the size of the repository is significant and the copy time is non_trivial (thus even in the build() method self.source_folder must be used)

Failure when using parent folder

For example assuming a folder structure as follows:

Consider the following conanfile (which conforms to the parent folder guidance):

from conan import ConanFile
from conan.tools.files import save, load

class BasicConanfile(ConanFile):
    name = "conan_source_bug"
    version = "0.1"
    description = "Demonstrate a bug in conan source() method"
    no_copy_source = True
    
    def layout(self):
        self.folders.root = ".."
        self.folders.source = "src"
        self.folders.generators = "generated"
    
    def generate(self):
        save(self, "generated_file.txt", "content")
        
    def source(self):
        # Saves source to source_folder
        self.output.info(f"In the source() method the Source folder is: {self.source_folder}")
        save(self, "source_file.c", "content") #i.e. get source from somewhere

    def build(self):
        self.output.info(f"In the build() method the Source folder is: {self.source_folder}")
        load(self, f"{self.source_folder}/source_file.c") # i.e. a compiler looking for source files

If you attempt to run conan source -> conan install -> conan build it will fail because the build method has a different definition of source_folder.

Correct me if I'm misunderstanding something.

Use case for having a root that is a subdir instead of parent

We ran across a use case where it t was desired that the source() method and all other methods use a root that is a subfolder of the current directory. This is our preferred workspace setup:

• ParentDir
  • conanfile.py
  • root_folder
    • src
    • generated

From a unix "principle of least surprise" perspective, it seems root folder should function consistently whether it is a subdirectory or parent folder and I don't think it requires any additional code to support both cases.

The fix I submitted is agnostic to whether it's a parent or child folder (the above conanfile will also work with the submitted fix).

Let me know if I've missed anything!

[memsharded]

Thanks very much for the detailed response and the details to reproduce the issue even for parent root folders, that helps.

Indeed, the fix is good, and as you said it is agnostic to the specific layout, so I think it might be moved forward.
I am a bit concerned that this kind of change might break others flow, even if they were relying on buggy behavior, but it seems it is the right behavior and it could be declared a bug, so lets try to move it forward for the next release.

I'll follow the comments about the layout in another comment below, and lets follow up on the PR for details about the failing tests.

Many thanks!

[memsharded]

Regarding:

We ran across a use case where it t was desired that the source() method and all other methods use a root that is a subfolder of the current directory. This is our preferred workspace setup:

This would be a bit unusual layout, as for example many build systems mandate, strongly recommend, or it is the default standard to have some file in the root of the repo. So having a CMakeLists.txt in the root of the repo, in this case, as a sibling to the conanfile.py would be the expected layout for CMake, for example.

So the self.folders.root was born with a different need, the need to put conanfiles inside subfolders (still leaving the CMakeLists.txt in the root, and also for cases when using super-projects with multiple packages, like one package with one conanfile.py per folder, but they relying on some common shared code that was not an independent package, and also like the whole setup is inside a git repo, so that needs to be handled.

I am not saying that other cases will be banned for the future, but when these features are introduced they are developed and tested accordingly to the use cases that originated the feature in the first place. So for this case, of using self.folders.root for a different "child" use case, the feature might be not ready or problematic, we simply not know. Until this is kind of evaluated and tested a bit more thoroughly, the restrictions in the docs will be maintained. Maybe as it seems you are using it and you have already figured out the tests, you might be able to provide feedback and more tests to enable this. I'd suggest to do this in a separate PR, I'll give some hints in the PR.

[timviola-cmi]

This seems like a good approach, I agree with maintaining the guidance given in the documentation.

I am going to consider reworking our folder structure in accordance with that guidance for now and will open a PR if I decide to move forward with the alternate folder structure as I don't want to drive changes that are unnecessary.