conan-io / conan-package-tools

Conan Package Tools. Helps with massive package creation and CI integration (Travis CI, Appveyor...)
MIT License
166 stars 71 forks source link

C2A: Conan deprecates CONAN_USERNAME and CONAN_CHANNEL #407

Open jgsogo opened 5 years ago

jgsogo commented 5 years ago

Description of Problem, Request, or Question

After deprecating environament variables CONAN_USERNAME and CONAN_CHANNEL, I think that we should get rid of them in this repo too, it shouldn't be a reliable way to feed user/channel to packages built with cpt.

Link to deprecation notice in the docs: https://docs.conan.io/en/latest/reference/env_vars.html#conan-username-conan-channel

If we agree to remove these variables here, it would be nice to remove them from the conan new --ci-* generated files too.

uilianries commented 5 years ago

I'm not sure. I see many users running CPT on their companies, which doesn't reflect what we are doing on Conan Center Index. Probably we will break many users/companies dropping it. I vote for supporting both cases on CPT.

Croydon commented 4 years ago

Is username/channel deprecated in Conan itself? If not, then it should not be deprecated within this tool

It is a nice way to distinguish between e.g. testing/stable or different sources/remotes and I hope that it stays as an optional feature in Conan

jgsogo commented 4 years ago

Hi! We would love to hear about those use cases. For Conan 2.0 we really think that these envrionment variables should be deprecated, but if there are valid use cases we need to hear about them in a Conan issue so we can have the information when thinking about 2.0

uilianries commented 4 years ago

I hope that it stays as an optional feature in Conan

As optional it would be nice.

I remember when I was using Conan in my past job, we have more than one version of same package, because our teams were not aligned, for instance:

zlib/1.2.11@company/stable
zlib/1.2.11@uilianries/testing

Well, oblivious the company's package is the official, but let's I need change something which I don't want share now because is very specific for an embedded stuff and I don't have time to wait 2 days of review, so I create my fork and use my own flavor of that package.

Now thinking in similar case, we use only zlib/1.2.11@. People will need to configure the remote, but also it will not allow different packages, because we don't have user/channel to distinguish between Conan Center, Company and Custom.

I agree @Croydon. user/channel can be used as optional on Conan 2.0.

jgsogo commented 4 years ago

This is not about user/channel, but about Conan taking into account environment variables CONAN_USERNAME and CONAN_CHANNEL. For sure there will be a way to disambiguate, probably user/channel like it is now (optional)

El lun., 27 ene. 2020 22:35, Uilian Ries notifications@github.com escribió:

I hope that it stays as an optional feature in Conan

As optional it would be nice.

I remember when I was using Conan in my past job, we have more than one version of same package, because our teams were not aligned, for instance:

zlib/1.2.11@company/stable zlib/1.2.11@uilianries/testing

Well, oblivious the company's package is the official, but let's I need change something which I don't want share now because is very specific for an embedded stuff and I don't have time to wait 2 days of review, so I create my fork and use my own flavor of that package.

Now thinking in similar case, we use only zlib/1.2.11@. People will need to configure the remote, but also it will not allow different packages, because we don't have user/channel to distinguish between Conan Center, Company and Custom.

I agree @Croydon https://github.com/Croydon. user/channel can be used as optional on Conan 2.0.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/conan-io/conan-package-tools/issues/407?email_source=notifications&email_token=AAKXL6HN2XF3FP67S7ICPQDQ75HSRA5CNFSM4IXDPBJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKBEYYY#issuecomment-578964579, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKXL6DTI6G4IBAQGQDHBILQ75HSRANCNFSM4IXDPBJQ .

uilianries commented 4 years ago

as you know, both used to determine the user/channel on the CPT. It is the same situation, if I'm using the CPT + CI service to distribute internally, how should I configure it without CONAN_USERNAME and CONAN_CHANNEL?

jgsogo commented 4 years ago

The user will be able to do whatever they want in the Python code, for example:

from conans import ConanFile

class Pkg(ConanFile):
    python_requires = f"pyreq/0.1@{os.environ["CONAN_USERNAME"]}/{os.environ["CONAN_PASSWORD"]}"

    def build(self):
        v = self.python_requires["pyreq"].module.myvar  # v will be 123
        f = self.python_requires["pyreq"].module.myfunct()  # f will be 234
        self.output.info("%s,%s" % (v, f))

and for sure, we can use those variables in the CI files and use them to call Conan (imagine this is valid Travis/Appveyor/Jenkins):

conan create . name/version@${{ env.CONAN_USERNAME }}/${{ env.CONAN_PASSWORD }}


Deprecating those variables in Conan is only about removing them from these functions: https://github.com/conan-io/conan/blob/6e8bb89137a01e0c6c243e27b0134ff567364193/conans/model/conan_file.py#L174

Croydon commented 4 years ago

and for sure, we can use those variables in the CI files and use them to call Conan (imagine this is valid Travis/Appveyor/Jenkins):

The point is, if you use conan-package-tools you don't call conan create yourself. We need a way to give the CONAN_USERNAME and CONAN_CHANNEL information to conan-package-tools

Croydon commented 4 years ago

I believe there are still function parameters for this, but I believe for most other parameters conan-package-tools is supporting parameters AND environment variables. So why making a difference here?

jgsogo commented 4 years ago

conan-package-tools can still get those variables from the environment and pass them to Conan, that won't make a difference, and we will check that everything keeps working the same. For Conan 2.0 we will thing about removing other environment variables too (https://github.com/conan-io/conan/issues/5270).

Probably this all is a result of my poor explanation when I opened this issue: Conan is deprecating those variables, so conan-package-tools should read the environment and pass them explicitly to conan create or to the conan_api (I don't know the internals of this repo).

uilianries commented 4 years ago

Until Conan 2.0, we will need to support it. CPT uses Conan API, so it reads the env vars CONAN_USERNAME and CONAN_CHANNEL, and pass them to the api call.

jgsogo commented 4 years ago

If CPT is already reading those environment variables and passing them to Conan, then we can close this issue, but, having a quick look at the sources I can see that CPT is passing that information as environment to the docker running Conan here: https://github.com/conan-io/conan-package-tools/blob/2f7a5ac5b65c611a337829b39d0803eaed19411e/cpt/runner.py#L236

We can modify CPT to use user/channel in the command line, no need to wait for Conan 2.0

artem-kamyshev commented 4 years ago

I second uilianries here. As far as I understand the username/channel values itselves will not be deprecated, only environment variables.

This environment variables became an easy way to configure CI builds without passing arguments into python build scripts directly. We are in process of setting up a CI and now I'm puzzled on what's going to come in several months? There is no deprecation warning on the CPT readme (CPT github main page), so I found this issue totally by chance. It sounds disturbing if in near future having wrote all the CI scripts we will have to update all our github workflow files to pass these arguments explicitly.

It seems CPT can solve this problem under the hood itself, without making its users to rewrite CI scripts. This will support stable CPT API.

jgsogo commented 4 years ago

Hi! As you say, stability of the CPT API is independent of Conan itself. Conan can deprecate this environment variables in the sense that it won't take into account those values, but CPT can still take those variables from the environment an feed Conan with them. Nothing changes from the point of view of the CPT user.


Anyway, @artem-kamyshev , if you are working on a CI for your company I would recommend you not to use CPT. This project has one objective: generate binaries massively for a combination of settings and options and this is usually not the case of a company (it is for Conan Center and open source libraries).

In a company you usually have a defined set of profiles you want to build binaries for, and those profiles don't change often. You usually store a configuration in a repository with all the needed profiles and share it with your developers (and the CI) using conan config install command. Then, features that revisions and lockfiles should be taken into account for a successful CI, and those features are not relevant for CPT.

Here (https://github.com/conan-io/conan/issues/5553) you can find an issue (and an attached doc) where people is sharing knowledge about CIs, and we are planning to generate documentation with some best practices. Feel free to reach us in the repo itself or in Slack if you want to share/ask information about your CI.

Thanks!

artem-kamyshev commented 4 years ago

@jgsogo Thank you for the clarification! The link is also a helpful one. Speaking about CPT and CI, we don't build the whole CI on CPT, only part of it for requirements of our software. There's quite a few of them as well as platforms (at least linux x86, x86_64, windows, mac os and some arm platforms) and sometimes different compilers for each platform, so CPT comes very handy in here.