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

fix(android): clean/rebuild should release gradle file locks #11712

Merged
merged 6 commits into from Jun 17, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented May 19, 2020

JIRA:
https://jira.appcelerator.org/browse/TIMOB-27882

Summary:

  • Gradle daemon file locks prevented some build files from being deleted on Windows.
  • Modified app rebuild to stop gradle daemon on Windows only.
  • Modified module clean to stop gradle daemon on Windows only.

Hyperloop App Test:

  1. Install this PR's SDK on a Windows 10 machine.
  2. Download the hyperloop-examples project.
  3. Open "tiapp.xml" and remove version from hyperloop module entry.
    <module>hyperloop</module>
  4. Build with Titanium version 9.0.1.GA for Android.
  5. Build with this PR's SDK version for Android and verify it works.
  6. Build with Titanium version 8.3.1.GA for Android.
  7. Build with this PR's SDK version for Android and verify it works.

Module Clean Test:

  1. Install this PR's SDK on a Windows 10 machine.
  2. Download the source for ti.imagefactory.
  3. Open the Command Prompt.
  4. CD to folder: .\ti.imagefactory\android
  5. Enter: appc ti sdk select <sdk-version> (Enter this PR's SDK version.)
  6. Enter: appc run -p android --build-only
  7. Enter: appc ti clean
  8. Verify that the clean occurred without errors.
  9. Verify folders were deleted: .\android\build and .\android\dist
  10. Enter: appc run -p android
  11. Verify module was built and example app runs in emulator.

@build
Copy link
Contributor

build commented May 19, 2020

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

android/cli/lib/gradle-wrapper.js#L298 - android/cli/lib/gradle-wrapper.js line 298 – 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead. (node/no-deprecated-api)

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 3 tests have failed There are 3 tests failing and 703 skipped out of 7391 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.Network.HTTPClientprogress event (9)0.508
Error: expected 0.1787709497206704 to be above or equal 1
at Assertion.fail (/node_modules/should/cjs/should.js:275:13)
      at Assertion.value (/node_modules/should/cjs/should.js:356:9)
      at HTTPClient.xhr.onsendstream (/ti.network.httpclient.test.js:685:23)
ios.ipad.Titanium.Network.HTTPClientprogress event (13.5)1.133
Error: failed to retrieve large image: [object Object]
file:///Users/build/Library/Developer/CoreSimulator/Devices/51FCF37C-6A90-4E29-A1BD-CDD275A6E989/data/Containers/Bundle/Application/03C1709F-75B8-4EAD-AEB0-3DFB5D38FCE3/mocha.app/ti.network.httpclient.test.js:700:17
ios.iphone.Titanium.Network.HTTPClientprogress event (13.3)1.299
Error: failed to retrieve large image: [object Object]
file:///Users/build/Library/Developer/CoreSimulator/Devices/3226ED90-7F6D-4C66-89AE-3791DE88C4F4/data/Containers/Bundle/Application/67C75545-87AE-4CDD-8C62-963D7CE0D077/mocha.app/ti.network.httpclient.test.js:700:17

Generated by 🚫 dangerJS against 66ef8dd

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

From my previous comments, I believe my test project was in an initial bad state. Subsequent builds are successful.

Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

@jquick-axway The first test seems to be fine but on the Module Clean Test when I do a appc ti clean I see the following error:

[DEBUG]  Deleting  C:\Users\APPC\Downloads\ti.imagefactory-stable\android\build
(node:28180) UnhandledPromiseRejectionWarning: Error: EBUSY: resource busy or locked, unlink 'C:\Users\APPC\Downloads\ti.imagefactory-stable\android\build\local.properties'
    at Object.unlinkSync (fs.js:1052:3)
    at rimrafSync (C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\node_modules\fs-extra\lib\remove\rimraf.js:254:15)
    at C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\node_modules\fs-extra\lib\remove\rimraf.js:291:39
    at Array.forEach (<anonymous>)
    at rmkidsSync (C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\node_modules\fs-extra\lib\remove\rimraf.js:291:26)
    at rmdirSync (C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\node_modules\fs-extra\lib\remove\rimraf.js:281:7)
    at Object.rimrafSync [as removeSync] (C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\node_modules\fs-extra\lib\remove\rimraf.js:252:7)
    at C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\android\cli\commands\_cleanModule.js:42:7
    at Array.forEach (<anonymous>)
    at Object.run (C:\ProgramData\Titanium\mobilesdk\win32\9.1.0\android\cli\commands\_cleanModule.js:38:11)
(node:28180) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:28180) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Test Environment

Operating System
  Name                        = Microsoft Windows 10 Pro
  Version                     = 10.0.18363
  Architecture                = 64bit
  # CPUs                      = 4
  Memory                      = 32.0GB

Node.js
  Node.js Version             = 12.16.1
  npm Version                 = 6.13.4

Appcelerator CLI
  Installer                   = 5.0.0
  Core Package                = 8.0.0

Titanium CLI
  CLI Version                 = 5.2.2
  node-appc Version           = 0.2.49

Java Development Kit
  Version                     = 10.0.1_10

@jquick-axway
Copy link
Contributor Author

Updated PR:

  • Removed gradle "clean" task from last commit.
  • Now always stops gradle daemon on clean/rebuild, but only for Windows.
    • This appears to be the only full proof solution to remove file locks.
    • Now always logs this action. Need to since gradle might download libraries/tools which can take a long time.
  • Reverted hyperloop back to 5.0.3.
    • We don't need to stop the gradle daemon on hyperloop side since app build will stop it 1st.
  • Modified module clean task to perform all operations async.

@ssjsamir
Copy link
Contributor

@jquick-axway The Hyperloop app is running fine but when cleaning ti.image factory I get the following error (although the build folder empties)

 [DEBUG]  Deleting  C:\Users\APPC\Downloads\ti.imagefactory-stable\android\build
 [ERROR]  :An error occurred during clean after 2s 421ms
 [ERROR]  :EPERM: operation not permitted, lstat 'C:\Users\APPC\Downloads\ti.imagefactory-stable\android\build\build_android.log'

@jquick-axway
Copy link
Contributor Author

jquick-axway commented May 20, 2020

The ./build/build_android.log file is generated by our CLI. Hmm...

Edit:
It looks like the log file error has been happening for 2 years. The below PR actively writes to a log file under the "build" directory during a "clean".
#9807

@jquick-axway
Copy link
Contributor Author

@ewanharris is going to take over this PR from here. He is going to look into deleting all files under the "build" directory except for the log file since the CLI is actively writing to it. Meaning that a "clean" will no longer completely delete the "build" folder anymore. We might also make a CLI "clean" change as well. Stay tuned!

jquick-axway and others added 5 commits June 10, 2020 12:07
- Gradle daemon file locks prevented some build files from being deleted on Windows.
- Modified app rebuild to run gradle "clean" task or "stop" gradle deamon as a fallback.
  * Titanium 7.x.x and 8.x.x has gradlew files for proguard, but cannot run "clean" due to missing project files.
- Modified module clean to run gradle "clean" task.
- Modified hyperloop to v5.0.4 to run gradle "clean" task.
- No longer running gradle "clean" task since it doesn't release lock on "local.properties" file. Stopping daemon seems to be the only solution.
- Modified module clean to delete all files/folders async.
- The 5.0.4 change is not needed on Android for TIMOB-27882.
On Windows as we were trying to write to the log file and remove it at the same time this would cause an error
@ewanharris
Copy link
Collaborator

@ssjsamir The issue around cleaning a module should now be resolved

@ssjsamir ssjsamir self-requested a review June 17, 2020 15:52
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed: Tested using the two test cases mentioned in the Description above.

Test Environment

Operating System
  Name                        = Microsoft Windows 10 Pro
  Version                     = 10.0.18363
  Architecture                = 64bit
  CPUs                        = 4
  Memory                      = 31.9GB
Node.js
  Node.js Version             = 12.16.1
  npm Version                 = 6.13.4
Appcelerator CLI
  Installer                   = 5.0.0
  Core Package                = 8.0.0
Titanium CLI
  CLI Version                 = 5.2.2
  node-appc Version           = 0.2.49
Java Development Kit
  Version                     = 10.0.1_10
  Java Home                   = C:\Program Files\Java\jdk-10.0.1
NDK Version : 20.1.5948944

@sgtcoolguy sgtcoolguy merged commit a699bf5 into tidev:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants