Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subrepositories and plugin settings inherited from root #3005

Open
cemeceme opened this issue Dec 24, 2023 · 3 comments
Open

Subrepositories and plugin settings inherited from root #3005

cemeceme opened this issue Dec 24, 2023 · 3 comments

Comments

@cemeceme
Copy link
Contributor

This is not a specific issue and more of a general overview of the issues I have been having about subrepositories.

I think this is best described by an example:
Suppose I have a repository A which depends on plugin C and D as well as another please repository B.
Repository B also depends on plugin C, but has different settings set in it's .plzconfig file.
B also depends on D, however, it compiles the tool that D uses internally and uses that instead of the default that A would use.
Finally, B also depends on another plugin E.

From my understanding, currently please will ignore any .plzconfig settings that are not in the root project, with the exception of definitions in PluginDefinition tags. This means that in this example, repository B will inherit the same settings for plugin C as the root repository A. The same would also go for D, but now repository A has to know about the rule that builds the internal tool executable. As far as I can tell, there is no way to override settings for other plugins, to e.g. specify a different set of DefaultOptCppflags for cc_rules.

I have come across "test/plugins/nested_subrepo" in this repo, that seems to address this partially, but again, it provided no hint as to how to specify settings for other plugins that should not get overridden or handled differently.

Finally, it also appears that the tool E in our example also doesn't end up working as all plugins appear to need to be defined in the root repository A. The only mention of having to list transient dependencies like this seems to be in the documentation page "Third-party dependencies/Comparison to other systems" where it reads: "If you're coming from Gradle or Maven, it's a little more alien due to being less language-specific and requiring full transitive dependencies to be specified.". However, I am not sure if this is actually in reference to this behavior.

To put this all as a single paragraph issue:
One cannot use subrepos as you would a library unless your root build environment matches closely enough to that of the subrepo, including things like warning flags. That is, unless I am mistaken and the documentation does not give adequate information on how to achieve configurable libraries using .plzconfig files.

@Tatskaari
Copy link
Member

Tatskaari commented Dec 28, 2023

The plugin settings are either inherited from the host repo, or overridden by the subrepo. This is defined by the plugin e.g. DefaultOptFlags is inherited:
https://github.com/please-build/cc-rules/blob/master/.plzconfig#L47

This means that we will use the flags as defined in the host repo. I think this is probably what you want most of the time, because these flags can be dependant on the toolchain you're using. Maybe we need another config option called AdditionalOptFlags that is not inherited that allows B to provide any other flags that should be applied on top of the flags from the host repo? Would that help?

@izissise
Copy link
Contributor

izissise commented Jan 3, 2024

As a workaround for this issue, I've been doing something similar to this :

buildfile = text_file(
    name="BUILD.plz",
    visibility=["PUBLIC"],
    strip=True,
    content=f"""
        CONFIG["OS"] = '{goos}'
        CONFIG["ARCH"] = '{goarch}'
        CONFIG["GO"] = {{
            'PLEASE_GO_TOOL': '{please_go_tool_tl}',
            'GO_TOOL': '{go_tool}',
            'AR_TOOL': '{ar_tool}',
            'CC_TOOL': '{cc_tool}',
            'STRIP_TOOL': '{strip_tool}',
            'C_FLAGS': [],
            'LD_FLAGS': [],
            'BUILDMODE': '{buildmode}',
            'CGO_ENABLED': '{cgo_enabled}',
            'COVERAGEREDESIGN': False,
            'CPP_COVERAGE': False,
            'DEFAULT_STATIC': {default_static},
            'DELVE_TOOL': 'dlv',
            'PKG_INFO': True,
            'RACE': False,
            'TEST_ROOT_COMPAT': False,
            'VALIDATE_MODULE_VERSION': False,
            'IMPORT_PATH': None,
            'LEGACY_IMPORTS': None,
            'SPLIT_DEBUG_INFO': None,
            'REQUIRE_LICENCES': None,
            'STDLIB': None,
        }}
        # build file
    """,
)

repo = filegroup(
    name="srepo",
    srcs = [buildfile, tree]
)

subrepo(
    name=subrepo_name,
    dep=repo,
    package_root=f"pkg/{CONFIG.OS}_{CONFIG.ARCH}/{module}",
)

This create a BUILD file for the subrepo that will override every config keys first and then do the actual work, you can even completely remove PluginDefinition from .plzconfig

I still get this bug #2943 though

@cemeceme
Copy link
Contributor Author

cemeceme commented Jan 16, 2024

Sorry for the late response.

The issue I'm facing is that I have transient plugins that need different settings for other plugins.

In my case, I have a c++20 project, which depends on 2 other c++ projects using please. Namely, a library I wrote & mysql-connector-cpp (I wrote a diff file to add please support for it).
The issue is that the latter sub-repo only compiles with c++17 but I cant seem to define a .please file that overrides DefaultOptCppflags/DefaultDbgCppflags just for it.
Instead I had to define something like what izissise suggested and assign a variable to all compiler_flags arguments of cc_library within the sub-repo.

Further issues arise as mysql-connector-cpp also provides a modified version of the protobuf compiler, so even though any projects that depend on it should not care, they still have to define the please proto rules and their arguments in the root .plzconfig file.

While something like AdditionalOptFlags would help, a more plugin agnostic approach would be better. Ideally, the root repo would not have to define what tools/other plugins are needed for the sub-repos it uses. Especially for things like internal compilers that wouldnt be used outside of the sub-repo in the first place.

If it helps, I can also look into creating a public repo for the patch file I made for mysql-connector-cpp, though right now it only adds the minimum required please support to it (I have not tried to dynamically generate version strings etc. that their current cmake setup does).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants