Exclude a package when building all from sources by uilianries · Pull Request #8483 · conan-io/conan

uilianries · 2021-02-10T20:15:30Z

We have 3 different ways to execute it, to illustrate:

Build all from sources, except zlib:

conan install boost/1.75.0@ --build=!zlib --build=*
conan install boost/1.75.0@ --build=!zlib --build

Do not build anything from sources. --build= means '' package, which is invalid.

conan install boost/1.75.0@ --build=!zlib --build=

Changelog: Feature: Skip package when building all package from sources at once using --build=!<package> syntax.
Docs: conan-io/docs#2023
closes #8442

Refer to the issue that supports this Pull Request.
If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

^{Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.}

If Conan install uses --build= it means all packages must be built. If another package is listed too, it doesn't matter. e.g conan install foo/0.1@ --build= --build=bar, where bar is dependency. As --build= listed, all packages will be built from sources Signed-off-by: Uilian Ries <uilianries@gmail.com>

When passing --build= or --build they should be the same, but actually they are considered different. --build= means "", --build means None However it's not intuitive if you are building a package: conan install <ref> --build OR conan install <ref> --build= Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2021-02-10T20:18:54Z


        assert isinstance(params, list)
-        if len(params) == 0:
+        if len(params) == 0 or "*" in params or "" in params:


If --build, --build= or --build=* is passed, build ALL from sources.

uilianries · 2021-02-10T20:20:19Z

                dest.extend(values)
            except ValueError:
                dest.append(values)
+        elif namespace == argparse.Namespace(build=[]):


--build= should be consider equal to --build. Otherwise, --build --build=foo will be parsed as --build=foo only.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded

While I understand the current behavior is not consistent, and it would be better to fix it, I think that this will be an issue, breaking users that for some reason rely on the current behavior, and unveiling other issues. Needing to do patch releases one after the other, etc. The same as when we changed the ignore_case=True in the FileCopier, which after several fixes still have some open issues. And it was "right" to fix it, but not worth the effort.

I would say that this case is similar. It is better to wait until Conan 2.0 to resolve these ambiguities (I haven't seen users reporting this), and if we want to implement now the feature request that the title is suggesting: Exclude a package when building all from sources, that might still be possible introducing the --build=!pattern, or --build-exclude=pattern or something like that.

uilianries · 2021-02-11T13:07:33Z

Indeed. Let's keep it for 2.0.

While --build=!pattern is more intuitive, our current argparse usage does not allow it. If we pass --build --build=anything, the argparse returns ['anything'], it discards --build because is None. However, if we pass --build= --build=anything, argparse returns ['anything', ''], which fits for --build=!anything.

So to avoid this kind of confusion, I would prefer a new argument, as you mentioned --build-exclude.

UPDATE:

Thinking again, we can accept --build=!foo too, it doesn't matter. If we pass --build --build=!foo or --build --build-exclude=foo, argparse will ignore --build.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2021-02-11T18:14:08Z

            except ValueError:
                dest.append(values)
+        # --build --build=foo == ["*", "foo"]
+        elif hasattr(namespace, "build"):


Without it, we won't be able to know if --build has been passed together with another --build=foo or --build=!foo. Argparse excludes None entries. As '*' is considered build all from sources and --build has the same usage, I don't think it will break Conan contract.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded · 2021-02-22T10:10:10Z

+        elif hasattr(namespace, "build") and isinstance(namespace.build, list):
+            if len(namespace.build) == 0 or \
+               any(str(it).startswith("!") for it in namespace.build):
+                dest.append("*")


Having a exclude pattern shouldn't be tied to building from source everything. You could have --build=mypkgs* --build=!mypkgs_special and it should build all packages starting with mypkgs but not the excluded one.

Please try to avoid as much as possible to particularize a generic thing as Extender with custom logic from one of its consumer, it is an anti-pattern and should be used only in case of high necessity.

I also think this might still be changing behavior, and this will probably bring issues, better to avoid it.

By default, when we pass --build --build=!foo, our parse only returns !foo, so doesn't matters whether --build is listed as command argument or not. This new conditional, only accepts --build=! when --build is present too. Or, we can create an implicit rule, like if you passed only --build=!foo, it means build all from sources, except for foo.

I would prefer it to be explicit, and move in the direction of Conan 2.0 CLI than adding it magically. So we could document that --build=!xxx requires other patterns to be positively defined, even if --build=*. Better than customize the Extender class with specific attribute logic (in other command --build could mean a different thing (actually it does in conan build --build command, and this is a risk of failure.

-build=!xxx requires other patterns to be positively defined

by contrary, we don't need to define an extra argument, without this workaround. If you agree, I'll remove it and keep only --build=!xxx, which is enough for us.

memsharded · 2021-02-22T10:10:56Z

            if self.never and (self.outdated or self.missing or self.patterns or self.cascade):
                raise ConanException("--build=never not compatible with other options")
-        self._unused_patterns = list(self.patterns)
+        self._excluded_patterns = [it for it in list(self.patterns) if it.startswith("!")]


Once the exclude_patterns has been computed, we don't need the ! anymore. It seems it would be better to clean it directly while computing self._excluded_patterns

My idea was printing to user that a package could be wrong named. If we don't keep that list, we can't provide such information.

What do you mean? I don't understand. We can keep that list, but in any case the ! is syntactic sugar, lets call it --build-exclude and that will remove all ambiguities.

Oh I see! Indeed if we use a new argument instead of !, we don't need to parse the value. Okay, if we will accept --build-exclude, I prefer accepting only it and excluding ! to avoid any ambiguous behavior.

memsharded · 2021-02-22T10:11:23Z

+                    fnmatch.fnmatchcase(repr(ref.copy_clear_rev()), pattern) or
+                    fnmatch.fnmatchcase(repr(ref), pattern))
+
+        for pattern in self.patterns:


Shouldn't this use self._excluded_patterns? Seems a bit confusing.

It's a possibility indeed, but the idea here is comparing if a pattern matches with excluded pattern, if so, exclude it from exclude patterns list and return False to say: Don't build this package from sources. All remaining patterns in excluded list will be printed to say: These packages don't match to a pattern to excluded.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2021-03-04T22:19:49Z

@memsharded I'm not going to say that we traded 6 for half a dozen, but add a new argument for --build-exclude and forwarding it, inflicted more files than before. Well, at least we didn't touch arg parser.

We still need exclude_pattern list, because only when checking "force", the package reference is available for validation. Take a look and tell me if you see a point which can be simplified. Honestly, I don't like to forward and forward a parameter just for a simple feature.

memsharded · 2021-03-29T15:49:19Z

@memsharded I'm not going to say that we traded 6 for half a dozen, but add a new argument for --build-exclude and forwarding it, inflicted more files than before. Well, at least we didn't touch arg parser.

We still need exclude_pattern list, because only when checking "force", the package reference is available for validation. Take a look and tell me if you see a point which can be simplified. Honestly, I don't like to forward and forward a parameter just for a simple feature.

Yes, I don't love this either... too much for such a feature. Need to re-think this one.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2021-04-20T14:36:58Z

@memsharded Good to go

* Build all package when --build= is listed If Conan install uses --build= it means all packages must be built. If another package is listed too, it doesn't matter. e.g conan install foo/0.1@ --build= --build=bar, where bar is dependency. As --build= listed, all packages will be built from sources Signed-off-by: Uilian Ries <uilianries@gmail.com> * Argument value None is a valid entry When passing --build= or --build they should be the same, but actually they are considered different. --build= means "", --build means None However it's not intuitive if you are building a package: conan install <ref> --build OR conan install <ref> --build= Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build parameter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Revert last 3 commits Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate graph build mode Signed-off-by: Uilian Ries <uilianries@gmail.com> * Add test for ignore build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Accept skipped build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix empty argument validation Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build message Signed-off-by: Uilian Ries <uilianries@gmail.com> * transform namespace.build in a list Signed-off-by: Uilian Ries <uilianries@gmail.com> * Improve build argument filter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Update --build help description Signed-off-by: Uilian Ries <uilianries@gmail.com> * conan-io#8442 Replace --build=! by --build-exclude Signed-off-by: Uilian Ries <uilianries@gmail.com> * fix broken arg * same behavior as --build --build=pattern * more test * conan-io#8442 Add --build=! command help Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: James <james@conan.io>

* wip * wip * working * remove dead unittest * CMake() needs CMakeToolchain * fix tests * minor refactor get_generator_platform * Return 'deprecated' attribute in 'conan inspect' command (#8832) * Return 'deprecated' attribute in 'conan inspect' command * fix tests * Exclude a package when building all from sources (#8483) * Build all package when --build= is listed If Conan install uses --build= it means all packages must be built. If another package is listed too, it doesn't matter. e.g conan install foo/0.1@ --build= --build=bar, where bar is dependency. As --build= listed, all packages will be built from sources Signed-off-by: Uilian Ries <uilianries@gmail.com> * Argument value None is a valid entry When passing --build= or --build they should be the same, but actually they are considered different. --build= means "", --build means None However it's not intuitive if you are building a package: conan install <ref> --build OR conan install <ref> --build= Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build parameter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Revert last 3 commits Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate graph build mode Signed-off-by: Uilian Ries <uilianries@gmail.com> * Add test for ignore build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Accept skipped build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix empty argument validation Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build message Signed-off-by: Uilian Ries <uilianries@gmail.com> * transform namespace.build in a list Signed-off-by: Uilian Ries <uilianries@gmail.com> * Improve build argument filter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Update --build help description Signed-off-by: Uilian Ries <uilianries@gmail.com> * #8442 Replace --build=! by --build-exclude Signed-off-by: Uilian Ries <uilianries@gmail.com> * fix broken arg * same behavior as --build --build=pattern * more test * #8442 Add --build=! command help Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: James <james@conan.io> * fix develop * Check Artifactory service context for conan_build_info --v2 (#8826) * check service context * add unittests * fix import * use json file! Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: Carlos Zoido <mrgalleta@gmail.com>

* wip * wip * working * remove dead unittest * CMake() needs CMakeToolchain * fix tests * minor refactor get_generator_platform * Return 'deprecated' attribute in 'conan inspect' command (conan-io#8832) * Return 'deprecated' attribute in 'conan inspect' command * fix tests * Exclude a package when building all from sources (conan-io#8483) * Build all package when --build= is listed If Conan install uses --build= it means all packages must be built. If another package is listed too, it doesn't matter. e.g conan install foo/0.1@ --build= --build=bar, where bar is dependency. As --build= listed, all packages will be built from sources Signed-off-by: Uilian Ries <uilianries@gmail.com> * Argument value None is a valid entry When passing --build= or --build they should be the same, but actually they are considered different. --build= means "", --build means None However it's not intuitive if you are building a package: conan install <ref> --build OR conan install <ref> --build= Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build parameter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Revert last 3 commits Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate graph build mode Signed-off-by: Uilian Ries <uilianries@gmail.com> * Add test for ignore build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Accept skipped build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix empty argument validation Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build message Signed-off-by: Uilian Ries <uilianries@gmail.com> * transform namespace.build in a list Signed-off-by: Uilian Ries <uilianries@gmail.com> * Improve build argument filter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Update --build help description Signed-off-by: Uilian Ries <uilianries@gmail.com> * conan-io#8442 Replace --build=! by --build-exclude Signed-off-by: Uilian Ries <uilianries@gmail.com> * fix broken arg * same behavior as --build --build=pattern * more test * conan-io#8442 Add --build=! command help Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: James <james@conan.io> * fix develop * Check Artifactory service context for conan_build_info --v2 (conan-io#8826) * check service context * add unittests * fix import * use json file! Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: Carlos Zoido <mrgalleta@gmail.com>

* removing gcc, visual, and msbuild old generators * removing more deprecated stuff * fixing more tests * fix export manifests test * removing more legacy * fixing tests * fix test (#8840) * Feature/cmake vs toolsets (#8815) * wip * wip * working * remove dead unittest * CMake() needs CMakeToolchain * fix tests * minor refactor get_generator_platform * Return 'deprecated' attribute in 'conan inspect' command (#8832) * Return 'deprecated' attribute in 'conan inspect' command * fix tests * Exclude a package when building all from sources (#8483) * Build all package when --build= is listed If Conan install uses --build= it means all packages must be built. If another package is listed too, it doesn't matter. e.g conan install foo/0.1@ --build= --build=bar, where bar is dependency. As --build= listed, all packages will be built from sources Signed-off-by: Uilian Ries <uilianries@gmail.com> * Argument value None is a valid entry When passing --build= or --build they should be the same, but actually they are considered different. --build= means "", --build means None However it's not intuitive if you are building a package: conan install <ref> --build OR conan install <ref> --build= Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build parameter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Revert last 3 commits Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate graph build mode Signed-off-by: Uilian Ries <uilianries@gmail.com> * Add test for ignore build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Accept skipped build package Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix empty argument validation Signed-off-by: Uilian Ries <uilianries@gmail.com> * Validate build message Signed-off-by: Uilian Ries <uilianries@gmail.com> * transform namespace.build in a list Signed-off-by: Uilian Ries <uilianries@gmail.com> * Improve build argument filter Signed-off-by: Uilian Ries <uilianries@gmail.com> * Update --build help description Signed-off-by: Uilian Ries <uilianries@gmail.com> * #8442 Replace --build=! by --build-exclude Signed-off-by: Uilian Ries <uilianries@gmail.com> * fix broken arg * same behavior as --build --build=pattern * more test * #8442 Add --build=! command help Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: James <james@conan.io> * fix develop * Check Artifactory service context for conan_build_info --v2 (#8826) * check service context * add unittests * fix import * use json file! Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: Carlos Zoido <mrgalleta@gmail.com> * fix new environment files with spaces in path (#8847) * fixed win tests Co-authored-by: Carlos Zoido <mrgalleta@gmail.com> Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com>

uilianries added 2 commits February 10, 2021 15:40

uilianries commented Feb 10, 2021

View reviewed changes

uilianries changed the title ~~[WIP] Exclude a package when building all from sources~~ Exclude a package when building all from sources Feb 10, 2021

uilianries marked this pull request as draft February 10, 2021 20:20

Validate build parameter

767f854

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded reviewed Feb 11, 2021

View reviewed changes

memsharded self-assigned this Feb 11, 2021

uilianries added 4 commits February 11, 2021 10:49

Revert last 3 commits

4e94449

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Validate graph build mode

f4bfd1b

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Add test for ignore build package

96ede96

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Accept skipped build package

210b8c9

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries marked this pull request as ready for review February 11, 2021 18:11

uilianries commented Feb 11, 2021

View reviewed changes

uilianries added 5 commits February 11, 2021 15:35

Fix empty argument validation

44e64ae

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Validate build message

4304f48

Signed-off-by: Uilian Ries <uilianries@gmail.com>

transform namespace.build in a list

de3ef30

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Improve build argument filter

2151c9f

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Update --build help description

e79eea8

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries mentioned this pull request Feb 12, 2021

Add --build=!skip for install command conan-io/docs#2023

Merged

memsharded requested changes Feb 22, 2021

View reviewed changes

uilianries added 2 commits March 4, 2021 19:11

conan-io#8442 Replace --build=! by --build-exclude

15a15f7

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Merge branch 'develop' into feature/skip-build-package

e55e19b

memsharded added 3 commits April 20, 2021 14:04

Merge branch 'develop' into feature/skip-build-package

6528be5

fix broken arg

eaa884e

same behavior as --build --build=pattern

a3c8855

memsharded and others added 2 commits April 20, 2021 15:15

more test

709217a

conan-io#8442 Add --build=! command help

2e4ce6e

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded approved these changes Apr 20, 2021

View reviewed changes

memsharded added this to the 1.36 milestone Apr 20, 2021

memsharded merged commit 4c89ed0 into conan-io:develop Apr 20, 2021

memsharded mentioned this pull request Apr 20, 2021

[question] Is there a way to skip specific packages from build when calling install --build? #8442

Closed

1 task

michaelmaguire mentioned this pull request Jun 7, 2021

Correct --build=! examples to show that --build didn't work, while --build=* did conan-io/docs#2119

Merged

memsharded mentioned this pull request Feb 18, 2022

Unify reference pattern matching #10587

Merged

Conversation

uilianries commented Feb 10, 2021 • edited by czoido Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uilianries Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

uilianries commented Feb 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

uilianries commented Mar 4, 2021

Uh oh!

memsharded commented Mar 29, 2021

Uh oh!

uilianries commented Apr 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uilianries commented Feb 10, 2021 •

edited by czoido

Loading

uilianries Feb 10, 2021 •

edited

Loading

uilianries commented Feb 11, 2021 •

edited

Loading