Skip to content

Conversation

bsansouci
Copy link
Contributor

Tell me what you think.

I'm basing this off of what ReProcessing needs. We'd also need a way to switch at compile time. You implemented some cppo-like feature but to my understanding it doesn't work with Reason (since it's another -pp?). @Schmavery and I have been using matchenv and for that we need to be able to set an environment variable per target. I'm not sure this that's the best way to do that so I didn't include in this PR the ability to set env-vars per target.

Copy link
Member

@bobzhang bobzhang left a comment

Choose a reason for hiding this comment

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

it would be nice that you have a working example in the comments, thanks

"type": "string",
"description": "The name of the target"
},
"compilationKind": {
Copy link
Member

Choose a reason for hiding this comment

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

can we have a short name, since it is already in the context of target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"targetItems": {
"type": "object",
"properties": {
"outputName": {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand, how it is different from compilation kind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah killing this.

"description": "The compiler to use for the target"
},
"entryPoint": {
"type": "string",
Copy link
Member

Choose a reason for hiding this comment

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

how does it work with library, library does not have an entry point?

Copy link
Member

Choose a reason for hiding this comment

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

entryPoint only makes sense for native backend, can we make the schema enforce such invariant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done following your advice.

@bsansouci
Copy link
Contributor Author

bsansouci commented Feb 25, 2017

The way this will work with libraries is simply that bsb will ignore those targets while building a library, and will only use the dependency tree and the root project's targets to decide how to build a project.

So those targets are either used for example code in a library, or for a real project (not a lib). But if bsb ignores them when building, someone could push their package to npm (whether it's a lib or not) and people could use it without conflict (with everything module public by default). This makes a project and a library the "same thing" from the consumer's perspective :D

Remove outputname as it can be simply the target kind.

Impose the invariant that JS backend doesn't need an entry point.
@bobzhang
Copy link
Member

@bsansouci this looks good to me, do you want to get this merged or wait together for a working implementation?

@bsansouci
Copy link
Contributor Author

I'd like to have a prototype of the implementation first. I should get one done tonight or tomorrow. Just to make sure this schema is actually usable.

@bobzhang
Copy link
Member

bobzhang commented Feb 28, 2017 via email

bobzhang added a commit that referenced this pull request Sep 20, 2017
253e94c v1.8.2
87111bf mark this 1.8.2.git
17ae02a Merge pull request #1323 from bradking/tolerate-phony-self-reference
c0dde0f Restore tolerance of self-referencing phony build statements
e679202 Factor ManifestParser options into a structure
f69c785 v1.8.1
7738c19 mark this 1.8.1.git
6081e81 Merge pull request #1318 from atetubou/fix_for_maxpath_test
480f899 fix normalizer test for _MAX_PATH
0c67152 v1.8.0
b98941a mark this 1.8.0.git
bfa7da5 Merge pull request #1313 from adzenith/patch-3
324a117 Add _Available since Ninja 1.4._ to `deps` and `recompact`
327d6ce Merge pull request #1314 from atetubou/fix_windows_path
86f606f Remove path component limit from input of CanonicalizePath in windows
e6e4984 Add `deps` and `recompact` tools to manual
7bbc708 Merge pull request #1111 from bradking/detect-cycles-early
61e347e Merge pull request #1292 from gtristan/work-around-zero-mtime
721d2a2 Drop unnecessary cycle detection in Plan::AddTarget
b6f020d Teach RecomputeDirty to detect cycles in the build graph
a8b5cdc Add infrastructure for efficient walks through the `Edge` graph
afe3beb Refactor RecomputeDirty to take a node instead of an edge
29a6e2f Simplify BuildTest.StatFailureAbortsBuild test case
b34f744 Work around mtime being set to 0 sometimes
1029064 Merge pull request #1291 from colincross/fix1290
11a934d Fix segfault on edge with no inputs
00f69c1 Merge pull request #1156 from cdbennett/windows-binary-mode-output
0142023 Merge pull request #1287 from ivadasz/dragonfly_patches
89d9d62 Merge pull request #1281 from colincross/rebuild_on_error
d806aa5 Add support for DragonFly.
04d886b Always rebuild on errors
a127dda Add a test that fails if a rule updates it output file but fails
036003d Move stat metric to DiskInterface
d6eb8ba Merge pull request #1280 from RedBeard0531/zsh_completion_explicit_file
ad54532 Make zsh completion use explicitly specified ninja files
2fc5b9e Revert 78f893b
f1551b3 Merge pull request #1271 from atetubou/faster_clparser
f0bb90e Reduce GetFullPathName calls
75b3385 Fix for review
3b32002 Make clparser faster
08a3220 Add string_piece_util
586bb6d Merge pull request #1267 from atetubou/clparser_perftest
5a03f4e delete n
03d0a8b use us
12ca621 add gitignore
4eac806 remove util.h
292c89f Add clparser_perftest
0b0374e Merge pull request #1255 from tchajed/bind-localhost
8a32d21 browse: Bind to localhost by default
fb3c700 Merge pull request #1231 from colincross/canon_perftest_fix
1df8b6e Merge pull request #1235 from refack/patch-1
f4ea9ca Merge pull request #1232 from nicolasdespres/fix-canon_perftest
4dc4f1d Merge pull request #1237 from danw/no_reload_with_restat
abcd5f3 Support restat when rebuilding manifest
d560c62 Need this to build on vs2017
d6ff22b Fix compilation error in canon_perftest.
d611bd4 Make travis build everything
110dae2 Fix build in canon_perftest_fix
2993752 Merge pull request #1181 from DanielWeber/issue-1161
9e71431 Merge pull request #1226 from colincross/close-fds
dd2b587 Close original pipe fd in subprocesses
a36f96c Merge pull request #1225 from bippum/patch-1
29ea8ea fix broken link in hacking.md
95ddaa1 Merge pull request #1220 from phajdan/master
76abf78 Fix build with uclibc
7d705a3 Merge pull request #1213 from nico/copying
78f893b replace copyright placeholder, fixes #1212
3082aa6 Merge pull request #1206 from nico/versioninfo
32c82da windows: replace deprecated GetVersionEx with recommended replacement
2263f82 Merge pull request #1205 from nico/clangclformat
32c3625 fix a clang-cl -Wformat warning
256bf89 forgot to bump version in manual for 1.7.2 release
a232a7f Merge pull request #1204 from nico/vernum
14f4794 fix RELEASING wrt manual.asciidoc process
fdfff11 Merge pull request #1194 from ilor/depfile-empty-path
21fc120 Merge pull request #1182 from moosotc/master
ef0616e Merge pull request #1203 from nico/reldoc
474e87d Update RELEASING -- manual.asciidoc no longer has a version number
04f4bc5 Merge pull request #1201 from nico/singlecommand
8b7155b teach -t commands to optionally print only the final command
46d9a95 Improve error message when a depfile contains a bad path
1cc730d Use uint64_t for slash_bits consistently
5153ced Use POSIX_SPAWN_USEVFORK if available
c4b09e1 Allow more path components
b042580 Write subprocess output to stdout in binary mode

git-subtree-dir: vendor/ninja
git-subtree-split: 253e94c
@bsansouci
Copy link
Contributor Author

Too old, closing!

@bsansouci bsansouci closed this Mar 23, 2020
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.

2 participants