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

PiranhaObjC : Unclear usage documentation #50

Closed
Samhithgb opened this issue Jun 15, 2020 · 11 comments
Closed

PiranhaObjC : Unclear usage documentation #50

Samhithgb opened this issue Jun 15, 2020 · 11 comments
Labels
legacy-objc Issues/PR related to older PiranhaObjC implementation

Comments

@Samhithgb
Copy link

Samhithgb commented Jun 15, 2020

There seems to be a few things missing in Objective C documentation, like :

  1. How do we pass piranha.properties file or list of method names corresponding to feature flags (like in PiranhaJava)?
  2. What does 'FlagType' mean in the argument list?
  3. Is it possible to trigger piranha for the entire project at once, instead of passing each file?
@mkr-plse
Copy link
Contributor

mkr-plse commented Jun 15, 2020

There seems to be a few things missing in Objective C documentation, like :

  1. How do we pass piranha.properties file or list of method names corresponding to feature flags (like in PiranhaJava)?

Unlike PiranhaJava and PiranhaSwift, PiranhaObjC properties are hardcoded currently. See the TODO here. The implementation has to be updated. Once #39 lands, we can take this up. Filed #51.

  1. What does 'FlagType' mean in the argument list?

FlagType is used for denoting treatment, control, etc. If the flag is treated, then the treatment APIs for the flag will return true.

  1. Is it possible to trigger piranha for the entire project at once, instead of passing each file?

To optimize analysis time, I recommend doing a grep for flag names to find the affected files and run PiranhaObjC on them.

@Samhithgb
Copy link
Author

Samhithgb commented Jun 17, 2020

@mkr-plse Thanks for the response. I have another query actually :

  • It is not clear how to get started with ObjC as well. The documentation just days run :

./piranha-objc.sh <FileName> <FlagName> <FlagType>

But I get this error when running it :

./piranha-objc.sh: line 12: ./piranha-objc/bin/clang: No such file or directory

Is there something else that needs to be run/installed to get started?

Update : I tried using ./generate-piranha-artifact.sh, but get the following error (including complete log) :

BANLSHIMPI:piranha user$ ./generate-piranha-artifact.sh

  • BRANCH_SCHEME=swift-5.1-branch
  • PIRANHA_VERSION=0.0.5
    ++ pwd
  • PIRANHA_SRC=piranha/src
    ++ pwd
  • PIRANHA_DIR=piranha/piranha-objc
    ++ pwd
  • BASE_DIR=piranha
    ++ pwd
  • PROCESS_DIR=piranha/process
  • LLVM_PROJECT=/piranha/process/llvm-project
  • LLVM_BUILDDIR=piranha/process/llvm-project/llvm/build
  • DYLIB_OUTPUTDIR=piranha/process/llvm-project/llvm/build
  • mkdir piranha/process
  • mkdir /piranha/process/llvm-project
  • cd /piranha/process
  • git clone https://github.com/apple/swift.git
    Cloning into 'swift'...
    remote: Enumerating objects: 16, done.
    remote: Counting objects: 100% (16/16), done.
    remote: Compressing objects: 100% (16/16), done.
    remote: Total 1108473 (delta 7), reused 2 (delta 0), pack-reused 1108457
    Receiving objects: 100% (1108473/1108473), 568.89 MiB | 6.17 MiB/s, done.
    Resolving deltas: 10% (97879/902409)
    Resolving deltas: 10% (98641/902409)
    Resolving deltas: 100% (902409/902409), done.
    Updating files: 100% (19459/19459), done.
  • ./swift/utils/update-checkout --clone --scheme swift-5.1-branch
    Skipping icu on Darwin
    Skipping cmake on Darwin
    Skipping clone of 'swift', directory already exists
    Skipping clone of 'icu', requested by user
    Skipping clone of 'cmake', requested by user
    Running obtain_additional_swift_sources with up to 8 processes.
    Traceback (most recent call last):
    File "./swift/utils/update-checkout", line 15, in
    update_checkout.main()
    File "/piranha/process/swift/utils/update_checkout/update_checkout/update_checkout.py", line 578, in main
    clone_results = obtain_all_additional_swift_sources(args, config,
    File "/piranha/process/swift/utils/update_checkout/update_checkout/update_checkout.py", line 344, in obtain_all_additional_swift_sources
    return run_parallel(
    File "/piranha/process/swift/utils/update_checkout/update_checkout/update_checkout.py", line 50, in run_parallel
    pool = Pool(processes=n_processes, initializer=init, initargs=(lk,))
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/context.py", line 119, in Pool
    return Pool(processes, initializer, initargs, maxtasksperchild,
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/pool.py", line 212, in init
    self._repopulate_pool()
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/pool.py", line 303, in _repopulate_pool
    return self._repopulate_pool_static(self._ctx, self.Process,
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/pool.py", line 326, in _repopulate_pool_static
    w.start()
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/context.py", line 283, in _Popen
    return Popen(process_obj)
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/popen_spawn_posix.py", line 32, in init
    super().init(process_obj)
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/popen_fork.py", line 19, in init
    self._launch(process_obj)
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/popen_spawn_posix.py", line 47, in _launch
    reduction.dump(process_obj, fp)
    File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/multiprocessing/reduction.py", line 60, in dump
    ForkingPickler(file, protocol).dump(obj)
    AttributeError: Can't pickle local object 'run_parallel..init'

BANLSHIMPI:piranha User$ ./generate-piranha-artifact.sh

  • BRANCH_SCHEME=swift-5.1-branch
  • PIRANHA_VERSION=0.0.5
    ++ pwd
  • PIRANHA_SRC=/piranha/src
    ++ pwd
  • PIRANHA_DIR=/piranha/piranha-objc
    ++ pwd
  • BASE_DIR=/piranha
    ++ pwd
  • PROCESS_DIR=/piranha/process
  • LLVM_PROJECT=/piranha/process/llvm-project
  • LLVM_BUILDDIR=/piranha/process/llvm-project/llvm/build
  • DYLIB_OUTPUTDIR=/piranha/process/llvm-project/llvm/build
  • mkdir /piranha/process
    mkdir: /piranha/process: File exists

@mkr-plse
Copy link
Contributor

./swift/utils/update-checkout --clone --scheme swift-5.1-branch is failing and the clang plugin is not being built. Let me get back to you on this.

@mkr-plse mkr-plse added the legacy-objc Issues/PR related to older PiranhaObjC implementation label Jun 17, 2020
@Samhithgb
Copy link
Author

Hi @mkr-plse Thanks a lot.

FYI, I was able to setup Piranha on a different machine without the above error.

Will the tool be able to parse iOS objC project files, with their imports spread across multiple packages/folders also? When trying out the tool on my project, and getting error :

(base) USER3:piranha User$ ./piranha-objc.sh FeatureManager.m feature_flag_name treatment
In file included from /Users/User/Documents/project/folder/featuremanager/FeatureManager.m:7:
/Users/User/Documents/project/folder/featuremanager/FeatureManager.h:6:9: fatal error: ‘TestHeader.h’ file not found
#import "TestHeader.h"
^~~~~~~~~~~~~~~~~~~~~

TestHeader.h is present in a different folder.

@mkr-plse
Copy link
Contributor

mkr-plse commented Jun 18, 2020

I don't think that should affect the refactoring. Can you do two things:

  1. Run test.sh as suggested here. Do the tests succeed?
  2. Then can you just add some simple code (according to how feature flags are implemented in your codebase) in one of the test files and see whether it is refactoring properly?

If both the steps above are successful and if the code in your codebase (FeatureManager.m) is not being refactored, I can try to create a reproducer with an unavailable header file to debug the issue.

Also, feel free to message in the gitter channel to discuss any intermittent issues.

@Samhithgb
Copy link
Author

Samhithgb commented Jun 19, 2020

@mkr-plse Thanks for the suggestions.

  1. Yes the test.sh is running fine.
  2. I added some code to your Treated.m, and seems to work fine as well. (The condition check is getting removed, and only log statement remains, as expected)

The issue is when I run it against my project file, the command fails with an error saying the header file is not found (as mentioned in the above comment). Is this a known issue? Please let me know. Thanks again! Will use the gitter channel for any further doubts.

@mkr-plse
Copy link
Contributor

@Samhithgb Confirming that I am able to reproduce this problem. By adding a random "TestHeader.h" in one of the test files, I get a

tests/OptimisticNamed.m:19:9: fatal error: 'TestHeader.h' file not found
#import "TestHeader.h"
        ^~~~~~~~~~~~~~

I will see what can be done to address this.

@mkr-plse
Copy link
Contributor

As a temporary fix, you can add the following to the piranha-objc.sh file.
-Xpreprocessor -I<path_to_header_file> after -fobjc-arc.
Let me know if this works for you.

The correct fix will be to call [SetSuppressIncludeNotFoundError](https://clang.llvm.org/doxygen/classclang_1_1Preprocessor.html#ac7bafe67fc32e41460855b39d20ff6af) from the Piranha implementation. I will file a separate issue for this.

@Samhithgb
Copy link
Author

Samhithgb commented Jun 20, 2020

Hey @mkr-plse, thanks for your quick response.

Adding header file path to the script maynot be a feasible option since we are planning to have an automated flow for this.

Sure, will be waiting for the above issue resolution then. Thanks.

@mkr-plse
Copy link
Contributor

Hey @mkr-plse, thanks for your quick response.

Sure, will be waiting for the above issue resolution then. Thanks.

@Samhithgb Once #67 is merged, can you verify whether you are able to run the refactoring.
You may still need to customize the exact API calls as part of the implementation.

@Samhithgb
Copy link
Author

Samhithgb commented Jun 24, 2020

@mkr-plse, I am unfortunately still facing this issue after taking in the recent changes. The error message is :

/Users/samhith/Project/featuremanager/FeatureManager.h:6:9: fatal error: 'WMExchangeAccount.h' file not found
#import "WMExchangeAccount.h"

Does the current fix also consider missing header files inside another header file? Please let me know if there is any workaround I can follow to avoid this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-objc Issues/PR related to older PiranhaObjC implementation
Projects
None yet
Development

No branches or pull requests

2 participants