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

[service] ninja windows binary outdated #2049

Closed
ericLemanissier opened this issue Jun 25, 2020 · 6 comments · Fixed by #2062
Closed

[service] ninja windows binary outdated #2049

ericLemanissier opened this issue Jun 25, 2020 · 6 comments · Fixed by #2062
Labels
blocked Affected by an external issue and waiting until it is solved bug Something isn't working infrastructure Waiting on tools or services belonging to the infra priority: high

Comments

@ericLemanissier
Copy link
Contributor

ericLemanissier commented Jun 25, 2020

there does not seem to be an up-to-date binary for ninja/1.10.0 on windows:
https://conan.io/center/ninja/1.10.0
https://bintray.com/conan/conan-center/ninja%3A_#files/_%2Fninja%2F1.10.0%2F_%2F3931d0d9c2d2df6551702798ae00538b%2Fpackage
https://github.com/turtlebrowser/turtlebrowser/runs/807869355?check_suite_focus=true#step:6:99

It's surprising because the last PR built correctly the 3 binaries: https://c3i.jfrog.io/c3i/misc/logs/pr/1985/1/ninja/1.10.0/

CC @patricia-gallardo

@jgsogo
Copy link
Contributor

jgsogo commented Jun 26, 2020

I've been investigating the PR logs... this is what I've seen:

image

but Windows is not built, it says:

============== Skipping configuration! ===============

Indeed, the profile chosen for Windows is:

[settings]
arch=x86_64
arch_build=x86_64
build_type=Debug
compiler=Visual Studio
compiler.runtime=MTd
compiler.version=14
os=Windows
os_build=Windows

and the ERROR in the outputs: https://c3i.jfrog.io/c3i/misc/logs/pr/1985/1/ninja/1.10.0/ca33edce272a279b24f87dc0d4cf5bbdcffbc187/

ERROR: ninja/1.10.0: Invalid configuration: Only MT MSVC runtime is supported

I'll copy/paste my comment here #1783 (comment):

Hi! Back here, first of all I want to apologize for introducing too much noise in my last comments (and more on this one). We've been talking about this issue because we had different opinions about these installers and we need to agree on how we want to handle them in Conan Center.

We think that the best approach is as follows:

Note.- Here I'm referring to packages that only contain executables, if they contain libraries that are consumed by other packages then they should be managed as regular packages. So, let's talk about pure installers, packages with executables only.

  1. We want these packages to show that Conan is able to retrieve the same binary for different configurations, that you don't need to build from sources for every change in the configuration (if all you want is an executable).
  2. These packages will list all the settings (os, arch, compiler, build_type) because all of them generate different artifacts, the package must compile and take into account all the combinations for these settings. We cannot raise a ConanInvalidConfiguration because it can block graphs with other packages in the build context. User building from sources must generate the binary that corresponds to given configuration.
  3. These packages will remove compiler and build_type from the package_id.
  4. Conan Center CI will iterate the profiles in a given order and will build the binaries for the first configuration that returns every different package-ID. This will ensure that these packages will be compiled with Release, static and a known compiler version. glibc shouldn't be a problem after Use base image for all compiler version conan-docker-tools#204 (probably before that we will use the lower compiler version to minimize this error)
  5. If these packages are required by others (because besides de executable it is generating other libraries) we will stop removing compiler and build_type in order to generate all the binaries again. Of course, we can consider other use-cases once they are known.

The main issue with this approach right now is that we cannot guarantee the order CCI uses to iterate the profiles, it means that we cannot be sure that the chosen configuration will be the one we expect. We are working on this, but it will take time (two weeks?). Meanwhile, we can stop this PR, or we can generate all these settings combinations and remove them afterward when the functionality required is implemented in CCI.

I hope this makes sense to everybody.

Thanks for pushing so hard!

There is no workaround until be iterate the profiles in the given order. Right now, moving the ConanInvalidConfiguration to the build() method is not the answer. It can happen that the chosen profile to build is one of the unsupported ones and no binaries will be generated.

Only solution right now is to generate all the binaries (all the valid configurations) and, once we iterate the profiles in order, we can recover those checks in the build() method.

Thanks for your understanding

@jgsogo
Copy link
Contributor

jgsogo commented Jun 26, 2020

ping @madebr @uilianries @SSE4 @danimtb ⬆️

@jgsogo jgsogo added blocked Affected by an external issue and waiting until it is solved bug Something isn't working infrastructure Waiting on tools or services belonging to the infra priority: high labels Jun 26, 2020
@ericLemanissier
Copy link
Contributor Author

ericLemanissier commented Jun 26, 2020

Do I understand correctly that the short term (temporary) solution implies not deleting settings.compiler from the package_id, even for executable only packages?

@jgsogo
Copy link
Contributor

jgsogo commented Jun 26, 2020

Yes, both, we need compiler and build_type (or Windows will choose debug) 😔

@ericLemanissier
Copy link
Contributor Author

It's not a perfect solution, but it's practical enough to unblock a lot of packages !

@danimtb
Copy link
Member

danimtb commented Jun 29, 2020

I am ok with that solution in the meantime if we agree that it should be reverted once we have the order of the profiles fixed in the pipeline unless there is any other reason to keep all the configurations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Affected by an external issue and waiting until it is solved bug Something isn't working infrastructure Waiting on tools or services belonging to the infra priority: high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants