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

b2: Support CXX from a VirtualBuildEnv #17882

Merged
merged 4 commits into from
Aug 11, 2023

Conversation

PeteAudinate
Copy link
Contributor

@PeteAudinate PeteAudinate commented Jun 9, 2023

Specify library name and version: b2/4.9.6

Parts of the b2 recipe read environment variables with os.environ.get(). This no longer works if the environment is provided from a VirtualBuildEnv.

Also, the b2 install step requires the correct compiler to be present, and will fail if the compiler isn't on the PATH. This is likely to be the case if the CXX environment variable is being used to specify the compiler binary. The best way to indicate the toolchain path to b2 seems to be to create a project-config.jam file with the path to the compiler (from the CXX flag).


@CLAassistant
Copy link

CLAassistant commented Jun 9, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

🤖 Beep Boop! This pull request is making changes to 'recipes/b2//'.

👋 @grafikrobot you might be interested. 😉

@PeteAudinate PeteAudinate changed the title B2 cxx env b2: Support CXX from a VirtualBuildEnv Jun 9, 2023
@PeteAudinate
Copy link
Contributor Author

To reproduce:

  • Remove g++ from your PATH
  • Create a test_profile containing:
    [buildenv]
    CXX=/path/to/g++
    
  • (for Conan v1) Run conan create . b2/4.9.3@ --profile test_profile --options b2:toolset=gcc --options b2:use_cxx_env=True

Without the first commit in this PR, build.sh fails to find g++.
Without the second commit in this PR, b2 install fails to find g++.

@PeteAudinate
Copy link
Contributor Author

@grafikrobot I'm not at all a b2 expert, so would appreciate your input :-). Is there a better way to do this?

@conan-center-bot

This comment has been minimized.

This ensures the b2 install command can find the correct compiler binary.
@conan-center-bot

This comment has been minimized.

This was referenced Jun 11, 2023
@ghost
Copy link

ghost commented Jun 11, 2023

I detected other pull requests that are modifying b2/portable recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@PeteAudinate
Copy link
Contributor Author

b2/4.3.0: Calling package()
ERROR: b2/4.3.0: Error in package() method, line 182
	copy(self, "*b2.exe", dst=self._pkg_bin_dir, src=self._b2_output_dir)
	OSError: [Errno 22] Invalid argument: 'C:\\J\\w\\prod-v2\\BuildSingleReference\\p\\t\\b21a65a25b24fc1\\b\\build\\output\\b2.exe'

@grafikrobot this failed once, then passed when I reran it above. I've also just seen this error with a local build (but again, it usually passes). Any idea what the cause might be? Should I raise a separate issue?

@grafikrobot
Copy link
Contributor

@PeteAudinate immediately I don't know why. I'd have to look at it in more detail to guess why. And I wont be able to look at all until after the 26th (I'm busy with the WG21 meeting and then out of contact on vacation).

@conan-center-bot

This comment has been minimized.

@PeteAudinate
Copy link
Contributor Author

Conan v1 pipeline ❌
Failure in build 3 (3674d44):

No idea why this got kicked off (or why it failed). It passed previously, and there have been no subsequent updates.
I've merged from master again to attempt a retry.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ghost ghost mentioned this pull request Jul 11, 2023
3 tasks
@AbrilRBS AbrilRBS self-assigned this Jul 12, 2023
@conan-center-bot conan-center-bot requested a review from jcar87 July 19, 2023 12:07
def _write_project_config(self, cxx):
with open(os.path.join(self.source_folder, "project-config.jam"), "w") as f:
f.write(
f"using {self.options.toolset} : : {cxx} ;\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for debug or confirm it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's writing to a project config file to tell b2 where the compiler is located. I'm a b2 novice, but this seemed like the best way to configure the project to use the compiler path from the CXX env var.

Without this, the b2 install stage uses the wrong compiler path.

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious does this work with tools.build:cxx in the conf?

@prince-chrismc prince-chrismc self-assigned this Jul 21, 2023
@PeteAudinate
Copy link
Contributor Author

I am curious does this work with tools.build:cxx in the conf?

Unfortunately not. The use_cxx_env option is specifically for instructing the recipe to respect the CXX/CXXFLAGS env vars. Having the ability to use the tools.build conf would be useful, but should probably be a separate feature (this PR is purely to fix a bug, and it's been waiting for a while to be merged so I'm a bit reluctant to add more functionality to it at this point :-))

@PeteAudinate
Copy link
Contributor Author

(that came out more passive-agressive than intended, I understand the demands on the team's time! 😄 )

@PeteAudinate
Copy link
Contributor Author

Raised in #18776

@prince-chrismc
Copy link
Contributor

That's perfectly fine! I was curious since you had a test vector it would be easy to check :)

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 3 (ab6afd4174c6c045be082868a81f9cce56713c45):

  • b2/4.3.0@:
    All packages built successfully! (All logs)

  • b2/4.7.1@:
    All packages built successfully! (All logs)

  • b2/4.4.0@:
    All packages built successfully! (All logs)

  • b2/4.4.2@:
    All packages built successfully! (All logs)

  • b2/4.4.1@:
    All packages built successfully! (All logs)

  • b2/4.8.1@:
    All packages built successfully! (All logs)

  • b2/4.10.0@:
    All packages built successfully! (All logs)

  • b2/4.9.1@:
    All packages built successfully! (All logs)

  • b2/4.8.0@:
    All packages built successfully! (All logs)

  • b2/4.8.2@:
    All packages built successfully! (All logs)

  • b2/4.9.5@:
    All packages built successfully! (All logs)

  • b2/4.5.0@:
    All packages built successfully! (All logs)

  • b2/4.6.0@:
    All packages built successfully! (All logs)

  • b2/4.7.2@:
    All packages built successfully! (All logs)

  • b2/4.6.1@:
    All packages built successfully! (All logs)

  • b2/4.9.0@:
    All packages built successfully! (All logs)

  • b2/4.9.4@:
    All packages built successfully! (All logs)

  • b2/4.9.6@:
    All packages built successfully! (All logs)

  • b2/4.9.3@:
    All packages built successfully! (All logs)

  • b2/4.10.1@:
    All packages built successfully! (All logs)

  • b2/4.9.2@:
    All packages built successfully! (All logs)

  • b2/4.7.0@:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds may be required once they are on the v2 ready list

All green in build 3 (ab6afd4174c6c045be082868a81f9cce56713c45):

  • b2/4.10.1@:
    All packages built successfully! (All logs)

  • b2/4.7.2@:
    All packages built successfully! (All logs)

  • b2/4.6.0@:
    All packages built successfully! (All logs)

  • b2/4.4.2@:
    All packages built successfully! (All logs)

  • b2/4.9.2@:
    All packages built successfully! (All logs)

  • b2/4.4.0@:
    All packages built successfully! (All logs)

  • b2/4.5.0@:
    All packages built successfully! (All logs)

  • b2/4.9.0@:
    All packages built successfully! (All logs)

  • b2/4.9.3@:
    All packages built successfully! (All logs)

  • b2/4.9.5@:
    All packages built successfully! (All logs)

  • b2/4.9.4@:
    All packages built successfully! (All logs)

  • b2/4.9.6@:
    All packages built successfully! (All logs)

  • b2/4.10.0@:
    All packages built successfully! (All logs)

  • b2/4.7.0@:
    All packages built successfully! (All logs)

  • b2/4.3.0@:
    All packages built successfully! (All logs)

  • b2/4.7.1@:
    All packages built successfully! (All logs)

  • b2/4.8.2@:
    All packages built successfully! (All logs)

  • b2/4.8.0@:
    All packages built successfully! (All logs)

  • b2/4.6.1@:
    All packages built successfully! (All logs)

  • b2/4.9.1@:
    All packages built successfully! (All logs)

  • b2/4.8.1@:
    All packages built successfully! (All logs)

  • b2/4.4.1@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 1485a37 into conan-io:master Aug 11, 2023
ericLemanissier pushed a commit to ericLemanissier/conan-center-index that referenced this pull request Sep 15, 2023
* Use environment from VirtualBuildEnv to get CXX/CXXFLAGS

* Write a project-config.jam file containing the toolset from CXX

This ensures the b2 install command can find the correct compiler binary.

---------

Co-authored-by: Chris Mc <christopherm@jfrog.com>
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

Successfully merging this pull request may close these issues.

7 participants