Skip to content

[Build] Make cmark build a build-script product #37102

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 22, 2021
Merged

[Build] Make cmark build a build-script product #37102

merged 1 commit into from
May 22, 2021

Conversation

drexin
Copy link
Contributor

@drexin drexin commented Apr 28, 2021

No description provided.

@drexin drexin marked this pull request as draft April 28, 2021 04:53
drexin
drexin commented Apr 28, 2021
View reviewed changes
@drexin
Copy link
Contributor Author

drexin commented Apr 28, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from 31dc1e8 to bb755ac Compare April 28, 2021 05:12
@drexin
Copy link
Contributor Author

drexin commented Apr 28, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch 2 times, most recently from 94a9344 to e8bfa86 Compare April 28, 2021 22:17
@drexin
Copy link
Contributor Author

drexin commented Apr 28, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from e8bfa86 to f8b7d28 Compare April 28, 2021 22:21
@drexin
Copy link
Contributor Author

drexin commented Apr 28, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from f8b7d28 to 64a553b Compare May 19, 2021 19:43
@drexin
Copy link
Contributor Author

drexin commented May 19, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from 64a553b to 3ab1dbf Compare May 20, 2021 23:33
@drexin
Copy link
Contributor Author

drexin commented May 20, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from 3ab1dbf to 21fe1ed Compare May 21, 2021 21:35
@drexin
Copy link
Contributor Author

drexin commented May 21, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from 21fe1ed to 3922708 Compare May 21, 2021 22:03
@drexin
Copy link
Contributor Author

drexin commented May 21, 2021

@swift-ci smoke test

@drexin drexin force-pushed the wip-build-script-products branch from 3922708 to ba99d98 Compare May 21, 2021 22:07
@drexin
Copy link
Contributor Author

drexin commented May 21, 2021

@swift-ci smoke test

gottesmm
gottesmm approved these changes May 21, 2021
View reviewed changes
Copy link
Contributor

@gottesmm gottesmm left a comment

Choose a reason for hiding this comment

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

Small bit of quick feedback. I am probably going to move those two methods to a parent class in a forthcoming commit. Looks good over all though!

utils/swift_build_support/swift_build_support/products/cmake_product.py
query_dir = os.path.join(self.build_dir, ".cmake", "api", "v1", "query")
if not os.path.exists(query_dir):
os.makedirs(query_dir)
open(os.path.join(query_dir, "codemodel-v2"), 'a').close()
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 really the canonical way to do this in python? I guess it works.

utils/swift_build_support/swift_build_support/products/cmake_product.py Outdated


class CMakeProduct(product.Product):
def should_include_host_in_lipo(self, host_target):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this method should be in the parent product class. Seems like something that might be useful for all products.

utils/swift_build_support/swift_build_support/products/cmake_product.py Outdated
return False

def host_install_destdir(self, host_target):
if self.args.cross_compile_hosts:
Copy link
Contributor

Choose a reason for hiding this comment

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

This method also I think should be in the parent product class. Seems like something that might be useful for all products.

@drexin drexin force-pushed the wip-build-script-products branch from ba99d98 to 346d1d0 Compare May 22, 2021 00:47
@drexin
Copy link
Contributor Author

drexin commented May 22, 2021

@swift-ci smoke test

gottesmm
gottesmm reviewed May 22, 2021
View reviewed changes
less Outdated
llvm
swift
swiftpm
Final Build Order:
Copy link
Contributor

Choose a reason for hiding this comment

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

@drexin I don't think you meant to commit this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dang it... thanks

@drexin drexin force-pushed the wip-build-script-products branch from 346d1d0 to c819493 Compare May 22, 2021 01:11
@drexin
Copy link
Contributor Author

drexin commented May 22, 2021

@swift-ci smoke test

@drexin
Copy link
Contributor Author

drexin commented May 22, 2021

@swift-ci smoke test macos

@drexin
Copy link
Contributor Author

drexin commented May 22, 2021

@swift-ci smoke test windows

1 similar comment
@drexin
Copy link
Contributor Author

drexin commented May 22, 2021

@swift-ci smoke test windows

@drexin drexin marked this pull request as ready for review May 22, 2021 16:00
@drexin drexin merged commit 3c19cc4 into swiftlang:main May 22, 2021
@drexin drexin deleted the wip-build-script-products branch May 22, 2021 16:04
benlangmuir added a commit that referenced this pull request May 23, 2021
@benlangmuir
Revert "[Build] Make cmark build a build-script product (#37102)"
3150086 
This reverts commit 3c19cc4.
@benlangmuir benlangmuir mentioned this pull request May 23, 2021
Merged
@benlangmuir
Copy link
Contributor

This is failing: https://ci.swift.org/job/oss-swift-package-macos/5874/console

17:24:52 CMake Error at cmake/modules/SwiftSharedCMakeConfig.cmake:200 (include):
17:24:52   include could not find load file:
17:24:52 
17:24:52     /Users/buildnode/jenkins/workspace/oss-swift-package-macos/build/buildbot_osx/cmark-macosx-arm64/src/cmarkTargets.cmake
17:24:52 Call Stack (most recent call first):
17:24:52   cmake/modules/SwiftSharedCMakeConfig.cmake:214 (swift_common_standalone_build_config_cmark)
17:24:52   CMakeLists.txt:521 (swift_common_standalone_build_config)

I'm going to revert it #37604. Please test with "swift-ci please build toolchain" before re-landing.

drexin added a commit to drexin/swift that referenced this pull request May 24, 2021
@drexin
Revert "Revert "[Build] Make cmark build a build-script product (swif…
7cfabf5 
…tlang#37102)""

This reverts commit 3150086.
LucianoPAlmeida pushed a commit to LucianoPAlmeida/swift that referenced this pull request May 27, 2021
@drexin @LucianoPAlmeida
[Build] Make cmark build a build-script product (swiftlang#37102)
ffaefb5
LucianoPAlmeida pushed a commit to LucianoPAlmeida/swift that referenced this pull request May 27, 2021
@benlangmuir @LucianoPAlmeida
Revert "[Build] Make cmark build a build-script product (swiftlang#37102
6a808e1 
)"

This reverts commit 3c19cc4.
LucianoPAlmeida pushed a commit to LucianoPAlmeida/swift that referenced this pull request May 27, 2021
@drexin @LucianoPAlmeida
Revert "Revert "[Build] Make cmark build a build-script product (swif…
c26953e 
…tlang#37102)""

This reverts commit 3150086.
DougGregor pushed a commit to DougGregor/swift that referenced this pull request Sep 17, 2021
@drexin @DougGregor
Revert "Revert "[Build] Make cmark build a build-script product (swif…
a61d116 
…tlang#37102)""

This reverts commit 3150086.

(cherry picked from commit 7cfabf5)
MaxDesiatov added a commit that referenced this pull request Nov 23, 2022
@MaxDesiatov
build-script-impl: remove CMark cases as unused
8722279 
CMark was converted from a `build-script-impl` product to `build-script` product in #37102. After that change remaining mentions of CMark in `build-script-impl` were unused and can be removed.
@MaxDesiatov MaxDesiatov mentioned this pull request Nov 23, 2022
Merged
MaxDesiatov added a commit that referenced this pull request Nov 23, 2022
@MaxDesiatov
build-script-impl: remove CMark cases as unused
f58964e 
CMark was converted from a `build-script-impl` product to `build-script` product in #37102. After that change remaining mentions of CMark in `build-script-impl` were unused and can be removed.
MaxDesiatov added a commit that referenced this pull request Nov 29, 2022
@MaxDesiatov
build-script-impl: remove CMark cases as unused
aaffc5f 
CMark was converted from a `build-script-impl` product to `build-script` product in #37102. After that change remaining mentions of CMark in `build-script-impl` were unused and can be removed.
MaxDesiatov added a commit that referenced this pull request Dec 7, 2022
@MaxDesiatov
Merge pull request #62234 from apple/maxd/cmark-build-cleanup
e80e7e3 
build-script-impl: remove CMark cases as unused

CMark was converted from a `build-script-impl` product to `build-script` product in #37102. After that change, some of the remaining mentions of CMark in `build-script-impl` were unused and can be removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gottesmm gottesmm gottesmm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@drexin @benlangmuir @gottesmm