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

Add mold 142 as build system changed from makefiles to cmake #12881

Merged
merged 31 commits into from
Oct 6, 2022

Conversation

AndreyMlashkin
Copy link
Contributor

@AndreyMlashkin AndreyMlashkin commented Sep 9, 2022

Specify library name and version: mold/1.4.2

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@@ -64,6 +64,9 @@ def _patch_sources(self):
files.replace_in_file(self, "source_subfolder/Makefile", "MOLD_LDFLAGS += -lmimalloc", "MOLD_LDFLAGS += -L{} -lmimalloc".format(
self.deps_cpp_info["mimalloc"].lib_paths[0]))

def build_requirements(self):
self.build_requires("cmake/3.24.1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need latest CMake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To keep it longer up-to-date. Is there any reason why I shouldn't use the latest version?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, not an issue. I just wanted to know why the default (the one installed in the CI) was not enough.

If it is needed only for new version 1.4.2, please add it only to its corresponding conanfile.py, not to old versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Build #2 failed because of cmake requrement
mold/1.4.2:
CMake Error at CMakeLists.txt:1 (cmake_minimum_required):
CMake 3.18 or higher is required. You are running version 3.15.7

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 seems, cmake shall got a version bump first... I hoped, I will get a new mold version fast

Copy link
Contributor

Choose a reason for hiding this comment

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

1.3.1 continues to build with make, you don't need cmake here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have separated versions 1.3.x and 1.4.x one is using make another is using cmake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I realized, you're right, it's not required for 1.3.x. Fixed now

@conan-center-bot

This comment has been minimized.

@ghost
Copy link

ghost commented Sep 9, 2022

I detected other pull requests that are modifying mold/all recipe:

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

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.



class MoldTestConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
# VirtualRunEnv can be avoided if "tools.env.virtualenv:auto_use" is defined
# (it will be defined in Conan 2.0)
generators = "VirtualBuildEnv"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@uilianries @prince-chrismc What sould it be in the end? VirtualBuildEnv or VirtualRunEnv? I am confused

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 stopped working, as I have applied a suggestion with this change.
Why on your oppinion, it should be Run environment and not build?

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@@ -1,3 +1,5 @@
versions:
"1.3.1":
folder: all
folder: 1.3.1
"1.4.2":
Copy link
Contributor

Choose a reason for hiding this comment

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

This got outdated since you included version 1.5. Would it make sense to rename the folder to all? So we have an all folder for current recipes and a legacy 1.3.1 folder. It looks like mold changes minor versions often since 1.4 is relatively new.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, makes sense

@conan-center-bot

This comment has been minimized.

@uilianries
Copy link
Member

@AndreyMlashkin Could you please add a test_v1_package? It's important to ensure Conan 1.x compatibility. Basically you just need to rename the previous version of test_package to test_v1_package.

@conan-center-bot
Copy link
Collaborator

All green in build 34 (5b3cb57ad44eb0ee9ca510f3a68a89882030ecd1):

  • mold/1.3.1@:
    All packages built successfully! (All logs)

    🔸 Informative: This recipe is not ready for Conan v2

    We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.
    This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.
    See the recipe migration guide to know more about the changes required.

    ERROR: Error loading conanfile at '/home/conan/w/prod_cci_PR-12881/recipes/mold/1.3.1/conanfile.py': Unable to load conanfile in /home/conan/w/prod_cci_PR-12881/recipes/mold/1.3.1/conanfile.py
      File "<frozen importlib._bootstrap_external>", line 728, in exec_module
      File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
      File "/home/conan/w/prod_cci_PR-12881/recipes/mold/1.3.1/conanfile.py", line 6, in <module>
        from conans import AutoToolsBuildEnvironment
    ImportError: cannot import name 'AutoToolsBuildEnvironment' from 'conans' (/opt/pyenv/versions/3.7.13/lib/python3.7/site-packages/conans/__init__.py)
    
  • mold/1.4.2@:
    All packages built successfully! (All logs)

  • mold/1.5.1@:
    All packages built successfully! (All logs)

self.output.info('Appending PATH environment variable: {}'.format(bindir))
self.env_info.PATH.append(bindir)
self.env_info.LD = mold_location
self.buildenv_info.prepend_path("MOLD_ROOT", bindir)
Copy link
Member

Choose a reason for hiding this comment

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

Now I see why it's not working with VirtuanRunEnv, there is no configuration for self.runenv_info but it's fine, as it's a linker, there is no reason for using on runtime environment.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@AndreyMlashkin AndreyMlashkin requested a review from SSE4 October 6, 2022 08:17
@conan-center-bot conan-center-bot merged commit 22d8d24 into conan-io:master Oct 6, 2022
@AndreyMlashkin
Copy link
Contributor Author

Johuuu, finally

@AndreyMlashkin AndreyMlashkin deleted the add_mold_142 branch October 6, 2022 19:18
Copy link
Contributor

@ericriff ericriff left a comment

Choose a reason for hiding this comment

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

LGTM

System-Arch pushed a commit to System-Arch/conan-center-index that referenced this pull request Oct 7, 2022
…to cmake

* add version 1.4.2

* add with_mimalloc option

* add validate method

* add cmake as build requrements

* use minmalloc from conan

* use minmalloc from conan

* use tbb from conan

* don't use cmake as requirement for old mold

* hotfix

* add cmake_find_package generator

* add CMakeDeps

* don't use old conans import

* Update recipes/mold/1.4.x/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* package licenses

* make artifacts match settings

* correct package method for mold 1.3.1

* package mold binary differently for gcc and clang

* exclude windows builds

* delete unneeded CMakeLists

* Apply suggestions from code review

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* import VirtualBuildEnv

* add a newline

* Apply suggestions from code review

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* delete package id also for 1.3.1 version

* use build_requirements instead of requirements in test

* Apply suggestions from code review

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/mold/1.4.x/test_package/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* make mold test recipe work again

* add version 1.5.1

* rename folder

* add test_v1_package

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.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