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.

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

Use cmake --build rather than directly invoking tools #54

Closed
willholen opened this issue Jul 18, 2019 · 2 comments
Closed

Use cmake --build rather than directly invoking tools #54

willholen opened this issue Jul 18, 2019 · 2 comments

Comments

@willholen
Copy link
Contributor

In several build scripts and build instructions, we directly invoke build specific tools, e.g. MSBuild.exe for MSVC++ and ninja for Ninja builds. We should instead use cmake --build for all of them.

This would simplify them by removing unnecessary dependencies and complexities, and allow using uniform configuration options like CMAKE_BUILD_PARALLEL_LEVEL.
@dulinriley dulinriley mentioned this issue Jul 26, 2019
Closed
@dulinriley
Copy link
Contributor

#66 fixes this for LLVM.
Currently configure.py for Hermes doesn't invoke any build command, so there's nothing to
change there, although we can certainly add it later.

@dulinriley
Copy link
Contributor

Whoops this actually wasn't merged yet, not sure why the PR is marked as closed

@dulinriley dulinriley reopened this Jul 29, 2019
facebook-github-bot pushed a commit that referenced this issue Aug 1, 2019
@dulinriley @facebook-github-bot
Use cmake --build instead of a separate build command (#66)
61d085a 
Summary:
Issue: #54

CMake already knows the correct build command based on the generator,
and can apply a consistent set of flags for things like parallelism.
CMake also presents a `--target` flag that makes it easy to select only one target,
so `--cross-compile-only` becomes unnecessary.
Pull Request resolved: #66

Reviewed By: willholen

Differential Revision: D16519117

Pulled By: dulinriley

fbshipit-source-id: ade342501b95f748f7c29d7c2a3b647f02fce104
mganandraj pushed a commit to mganandraj/hermes that referenced this issue Oct 15, 2019
@dulinriley @mganandraj
Use cmake --build instead of a separate build command (facebook#66)
cb7c3f6 
Summary:
Issue: facebook#54

CMake already knows the correct build command based on the generator,
and can apply a consistent set of flags for things like parallelism.
CMake also presents a `--target` flag that makes it easy to select only one target,
so `--cross-compile-only` becomes unnecessary.
Pull Request resolved: facebook#66

Reviewed By: willholen

Differential Revision: D16519117

Pulled By: dulinriley

fbshipit-source-id: ade342501b95f748f7c29d7c2a3b647f02fce104
facebook-github-bot pushed a commit that referenced this issue Mar 23, 2021
@passy @facebook-github-bot
Publish headers archive again (#54)
5141113 
Summary:
This broke with the switch to the new gradle release plugin.
Read the source of it and while I'm not sure if it's the right way to do
it, it seems to work.

Pull Request resolved: facebookincubator/fbjni#54

Test Plan:
```
$ ./gradlew installArchives
$ ls ~/.m2/repository/com/facebook/fbjni/fbjni/0.2.1-SNAPSHOT/
```

Contains a headers archive again.

```
unzip -l ~/.m2/repository/com/facebook/fbjni/fbjni/0.2.1-SNAPSHOT/fbjni-0.2.1-20210322.182921-1-headers.jar
Archive:  /Users/realpassy/.m2/repository/com/facebook/fbjni/fbjni/0.2.1-SNAPSHOT/fbjni-0.2.1-20210322.182921-1-headers.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  03-22-2021 18:29   META-INF/
       25  02-19-2021 12:28   META-INF/MANIFEST.MF
        0  01-23-2020 14:25   fbjni/
     2056  01-23-2020 14:25   fbjni/JThread.h
      951  01-23-2020 14:25   fbjni/ReadableByteChannel.h
     1336  01-23-2020 14:25   fbjni/NativeRunnable.h
     1050  01-23-2020 14:25   fbjni/File.h
     1088  01-23-2020 14:25   fbjni/fbjni.h
     1215  01-23-2020 14:25   fbjni/Context.h
     1759  01-23-2020 14:25   fbjni/ByteBuffer.h
        0  02-18-2021 11:31   fbjni/detail/
    11992  01-23-2020 14:25   fbjni/detail/Hybrid.h
     1156  01-23-2020 14:25   fbjni/detail/Meta-forward.h
     4905  02-18-2021 11:31   fbjni/detail/MetaConvert.h
     5607  01-23-2020 14:25   fbjni/detail/TypeTraits.h
     9358  02-18-2021 11:31   fbjni/detail/Registration.h
    19170  02-18-2021 11:31   fbjni/detail/CoreClasses-inl.h
    17339  01-23-2020 14:25   fbjni/detail/Meta-inl.h
     4573  07-31-2020 15:56   fbjni/detail/Exceptions.h
     2783  01-23-2020 14:25   fbjni/detail/Boxed.h
     5420  07-31-2020 15:56   fbjni/detail/Environment.h
     2110  01-23-2020 14:25   fbjni/detail/References-forward.h
     3042  01-23-2020 14:25   fbjni/detail/Common.h
     6938  07-31-2020 15:56   fbjni/detail/Registration-inl.h
     2361  01-23-2020 14:25   fbjni/detail/Log.h
      802  07-31-2020 15:56   fbjni/detail/FbjniApi.h
    14670  02-18-2021 11:31   fbjni/detail/Meta.h
    23860  07-31-2020 15:56   fbjni/detail/CoreClasses.h
    15507  01-23-2020 14:25   fbjni/detail/References-inl.h
     1563  01-23-2020 14:25   fbjni/detail/JWeakReference.h
    10912  02-18-2021 11:31   fbjni/detail/SimpleFixedString.h
     4017  07-31-2020 15:56   fbjni/detail/ReferenceAllocators-inl.h
     4577  01-23-2020 14:25   fbjni/detail/Iterator.h
     6276  01-23-2020 14:25   fbjni/detail/Iterator-inl.h
     3049  01-23-2020 14:25   fbjni/detail/utf8.h
    22713  02-18-2021 11:31   fbjni/detail/References.h
     1954  01-23-2020 14:25   fbjni/detail/ReferenceAllocators.h
        0  02-18-2021 11:31   lyra/
     3092  01-23-2020 14:25   lyra/lyra_exceptions.h
     6684  01-23-2020 14:25   lyra/lyra.h
---------                     -------
   225910                     40 files

```

Reviewed By: IvanKobzarev

Differential Revision: D27237590

Pulled By: passy

fbshipit-source-id: 8212623e4121684a48b8c965fa4f6efdb78b384d
@whatleo whatleo mentioned this issue Apr 1, 2021
Closed
mganandraj pushed a commit to mganandraj/hermes that referenced this issue Nov 29, 2021
@dannyvv
Merge pull request facebook#54 from dannyvv/pr/SwitchToPatch
ee7a549 
Add patch file for LLVM bugfix
@vietmobilefolk vietmobilefolk mentioned this issue Sep 22, 2022
Closed
@DevFuan DevFuan mentioned this issue Jan 12, 2023
Open
@atoui-zahreddine atoui-zahreddine mentioned this issue Feb 9, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dulinriley @willholen