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

Fix issues: 48, 49, 50, 53, 54, 57 #58

Merged
merged 7 commits into from
Apr 27, 2016
Merged

Fix issues: 48, 49, 50, 53, 54, 57 #58

merged 7 commits into from
Apr 27, 2016

Conversation

stefanb2
Copy link
Contributor

I hope this cumulative pull request is OK

BTW: the first commit adds my name to the CONTRIBUTORS file

Stefan Becker added 7 commits April 12, 2016 12:29
add Stefan to CONTRIBUTORS
cd060c5 
Change-Id: I4ee4ce5d72e855306546deee2b4e9c3c25694585
[C++] Add support for .POSIX:
187bf08 
Add a global flag that is set to "true" when .POSIX: is encountered. The
flag will switch the bash flags from "-c" to "-ec", i.e. enable correct
error handling of shell command lines.

Fixes google#48

Change-Id: Ieea386742b05b52d209b74e640e14212f0e2da88
[C++] Ignore white space around $(call) function name
167e1f7 
Regression when compared to GNU make behaviour.

Test case:

 $ cat Makefile.call-func-name
 func = $(info called with '$(1)')
 test = $(call $(1),$(1))

 $(call test,func)
 $(call test, func)
 $(call test,func )
 $(call test, func )

 $ make -f Makefile.call-func-name
 called with 'func'
 called with ' func'
 called with 'func '
 called with ' func '
 make: *** No targets.  Stop.

 $ ckati -c --warn -f Makefile.call-func-name
 called with 'func'
 Makefile.call-func-name:5: *warning*: undefined user function:  func
 Makefile.call-func-name:6: *warning*: undefined user function: func
 Makefile.call-func-name:5: *warning*: undefined user function:  func
 *** No targets.

Fixes google#49

Change-Id: I3d982cd8d6d9777034df64540c32846300cb72f2
[C++] Honor "override" when setting global variable
29b9b74 
Regression when compared to GNU make behaviour.

Test case:

 $ cat Makefile.override-failure
 $(info VAR: '$(VAR)')
 override VAR := test
 $(info VAR: '$(VAR)')
 override VAR := test-new
 $(info VAR: '$(VAR)')
 VAR := test-should-not-work
 $(info VAR: '$(VAR)')

 $ make -f Makefile.override-failure
 VAR: ''
 VAR: 'test'
 VAR: 'test-new'
 VAR: 'test-new'
 make: *** No targets.  Stop.

 $ ckati -c --warn -f Makefile.override-failure
 VAR: ''
 VAR: 'test'
 VAR: 'test'
 VAR: 'test'
 *** No targets.

Fixes google#50

Change-Id: I9c4185c30cfcf5602da7e0ac98b7e9c420788005
[C++] Store SHELL value in command result
d4f2871 
$(shell ...) command lines are executed using $(SHELL). We need to use
the same shell during regeneration check instead of hard-coding /bin/sh
or otherwise the results might be different when $(SHELL) is redefined
in the makefile.

Fixes google#53

Change-Id: I1f9809106f29f7e806324a82e2323a2f8df64b63
[C++] Replace erroneous return in EvalInclude()
786881c 
We need to continue the loop, not abort the function, otherwise kati
will ignore include files that should not be ignored.

Test case:

 $ cat Makefile.minus-include-main
 .PHONY: default
 default:

 -include Makefile.minus-include-?
 $ cat Makefile.minus-include-a
 $(info include a...)
 $ cat Makefile.minus-include-b
 $(info include b...)

 $ ckati --warn --gen_all_targets --regen --ninja -f Makefile.minus-include-main
 include b...
 include a...
 $ ckati --warn --gen_all_targets --regen --ninja -f Makefile.minus-include-main --ignore_optional_include=Makefile.minus-include-a
 arguments changed, regenerating...
 include b...
 $ ckati --warn --gen_all_targets --regen --ninja -f Makefile.minus-include-main --ignore_optional_include=Makefile.minus-include-b
 arguments changed, regenerating...

Fixes google#54

Change-Id: I139392510b02d48c224edf4a8e6e186d52f26699
[C++] Ignore recursive marker in recipes
952d445 
Test case:

$ cat Makefile.recursive
.PHONY: default
default:
        +echo DONE

$ ckati --warn --gen_all_targets --regen --ninja -f Makefile.recursive
Makefile.recursive was modified, regenerating...
$ fgrep DONE build.ninja
 command = /bin/sh -c "+echo DONE"

$ ninja
[1/1] build default
FAILED: default
/bin/sh -c "+echo DONE"
/bin/sh: + : invalid option
Usage:  /bin/sh [GNU long option] [option] ...
...

Fixes google#57

Change-Id: Ic40d71ff2b168158661eb2eac638adaeadaec1fe
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@stefanb2
Copy link
Contributor Author

I was added to our companys' corporate CLA a few weeks back for AOSP contributions:

I was under the impression that this would be enough to submit to GitHub too.

@stefanb2
Copy link
Contributor Author

I verified that I've added the email address stefanb@gpartner-nvidia.com to my GitHub account

@stefanb2 stefanb2 mentioned this pull request Apr 26, 2016
Closed
@shinh
Copy link
Contributor

shinh commented Apr 27, 2016

Great! Thanks for handling the CLA stuff. These changes look great, though I'd like to see tests in the testcase directory. You gave me clear test cases so I'll merge this PR and add tests by myself. I think I'll do a few follow-up changes, too. Thanks again!

@shinh shinh merged commit 4e60e5f into google:master Apr 27, 2016
shinh
shinh reviewed Apr 27, 2016
View reviewed changes
symtab.cc
@@ -18,6 +18,8 @@

#include "symtab.h"

#include <iostream>
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary

@stefanb2 stefanb2 deleted the topic-issue-57 branch April 27, 2016 07:16
shinh
shinh reviewed Apr 27, 2016
View reviewed changes
flags.h
@@ -34,6 +34,7 @@ struct Flags {
bool is_dry_run;
bool is_silent_mode;
bool is_syntax_check_only;
bool posix_shell;
Copy link
Contributor

Choose a reason for hiding this comment

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

This global state should be handled at evaluation time and it seems $(shell) should use this. I've moved this from here to eval.h. 2941ea0

zchee pushed a commit to zchee/kati that referenced this pull request Nov 29, 2016
@shinh
Merge pull request google#58 from stefanb2/topic-issue-57
87f840a 
Fix issues: 48, 49, 50, 53, 54, 57
zchee pushed a commit to zchee/kati that referenced this pull request Nov 29, 2016
@shinh
Merge pull request google#58 from stefanb2/topic-issue-57
d410dc1 
Fix issues: 48, 49, 50, 53, 54, 57
zchee pushed a commit to zchee/kati that referenced this pull request Nov 29, 2016
@shinh
Merge pull request google#58 from stefanb2/topic-issue-57
31c5e84 
Fix issues: 48, 49, 50, 53, 54, 57
danw pushed a commit to danw/kati that referenced this pull request Sep 15, 2018
@colincross
Merge remote-tracking branch 'aosp/upstream' into master
ff2d59e 
390115c Handle nested define/endef
c15a824 Update findleaves.py and add a few testcases
af13468 Merge pull request google#73 from colincross/findleaves
f0a6fdf [C++] Add support for multiple filenames to findleaves emulation
1c3a695 Support all kinds of command line variables
ac6f169 Allow NULL filename for -d flag
c58db9a [C++] Add -d flag to make debugging slightly easier
38892d8 Normalize log for recent ninja
9f6343c Always sort glob results
8082dcb Skip 3 tests which fail with make 4
1561f68 Skip shell_var_with_args.mk with GNU make 4
a1ded1c Use override in posix_var.mk to fix test with make 4
fc24bd2 Stop overwriting /bin/sh by bash on travis
913eb74 Normalize Unicode quotes for recent find
2d2ed95 Explicitly use SHELL=/bin/bash
d07e297 [C++] Stop using an uninitialized value
855fea4 Fix multi_implicit_output_patterns.mk for GNU make 4
dc258cb Clean up normalization in runtest.rb a bit
3009771 Merge pull request google#66 from stefanb2/topic-issue-65
bfae810 Remove test output in ckati_clean
a24a7a0 Normalize GNU make 4.00 in runtest.rb
e4e56f3 Suppress GNU make jobserver magic in runtest.rb
2941ea0 [C++] Handle .POSIX at eval time
03fa345 Add testcase/posix_var.mk
a8fafa4 Remove Go related targets from test and clean
d8d43ea [C++] Reduce unnecessary string allocation
c6926b0 Add testcase/call_with_whitespace.mk
5dfb361 Add testcase/recursive_marker.mk
02e5ee3 [C++] Remove unnecessary #include
6823600 Add a test case to override_override.mk
4e60e5f Merge pull request google#58 from stefanb2/topic-issue-57
952d445 [C++] Ignore recursive marker in recipes
786881c [C++] Replace erroneous return in EvalInclude()
d4f2871 [C++] Store SHELL value in command result
29b9b74 [C++] Honor "override" when setting global variable
167e1f7 [C++] Ignore white space around $(call) function name
187bf08 [C++] Add support for .POSIX:
cd060c5 add Stefan to CONTRIBUTORS

Change-Id: I7697703aa2a0eb37202645b1895899807e6426b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: no
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@stefanb2 @googlebot @shinh