-
Notifications
You must be signed in to change notification settings - Fork 534
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 #2659: Add third party wrapper libs #2663
Conversation
Note this might be specific to Bazel 4.0, but this seems more correct anyway.
I'm probably just going to migrate everything since I think this can be easily generalized.
Also, automatically generate the wrapper libraries. Only constraint layout is being used currently.
This adds more versions to a central location for management. It's not quite comprehensive, but it does provide at least a place for these versions to live as the codebase continues to change over time.
Also, fixes some references to artifacts that weren't actually being imported via Maven.
/cc @fsharpasharp as well. Apparently you weren't added as a collaborator yet (just sent the invite), so I can't assign you yet. Once you comment I should be able to though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR LGTM, just added some comments to get me more clearer How this can be useful for the "third-party" project.
sed 's/import \"/import \"model\/src\/main\/proto\//g' | | ||
sed 's/\"model\/src\/main\/proto\/exploration/\"model\/processed_src\/main\/proto\/exploration/g' | | ||
sed 's/\"model\/src\/main\/proto\/topic/\"model\/processed_src\/main\/proto\/topic/g' | | ||
sed 's/\"model\/src\/main\/proto\/question/\"model\/processed_src\/main\/proto\/question/g' > $@ | ||
sed 's/import \\"/import \\"model\\/src\\/main\\/proto\\//g' | | ||
sed 's/\\"model\\/src\\/main\\/proto\\/exploration/\\"model\\/processed_src\\/main\\/proto\\/exploration/g' | | ||
sed 's/\\"model\\/src\\/main\\/proto\\/topic/\\"model\\/processed_src\\/main\\/proto\\/topic/g' | | ||
sed 's/\\"model\\/src\\/main\\/proto\\/question/\\"model\\/processed_src\\/main\\/proto\\/question/g' > $@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for correcting this, probably it was causing an error while I was trying to run bazel query commands.
@BenHenning Please mark the above conversation resolved on my behalf as I can not resolve them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very neat solution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separation looks very clean!
Merging this since the comments seemed to be addressed |
Thanks all! |
This fixes some tests that were broken after recent PRs, and fixed a visibility error introduced in #2663.
…Oppia tests (#1904) * Update Bazel CI workflow Build all Oppia targets in Bazel rather than just the binary target. * Update main.yml Reset the workflow name so that it doesn't need to be renamed in GitHub settings. * Introduce remote caching in Bazel. This uses a remote storage service with a local file encrypted using git-secret to act as the authentication key for Bazel to read & write artifacts to the service for caching purposes. * Add debug line. * Disable workflows + fix debug line. * More debugging. * More debugging. * Work around GitHub hiding secret since we're debugging. * Use base64 to properly encode newlines in GPG keys. * Remove debug lines before changing back to correct GPG key. * Switch to production key. * Fix env variable reference. Lock-down actions workflows via codeowners. * Install git-secret to default location. * Add details. Debug $PATH. * Fix pathing for git-secret. * Dummy commit to re-trigger actions. * Undo commit to see if this one shows up. * Fix git-secret pathing try 2. * New commit to re-trigger action. * Path debugging. * Workaround to get GitHub to show changes. * Update runner to use Bash. Reference: https://github.community/t/github-path-does-not-add-to-the-path/143992. * Restore binary-only build, other builds, and introduce new workflow for building all Bazel targets. * Remove debug lines. * Rename & remove keep_going. * Compute matrix containing all test targets. This will allow us to distribute parallelization responsibilities partly to GitHub actions to hopefully have slightly better throughput. See https://github.blog/changelog/2020-04-15-github-actions-new-workflow-features/ for reference on how this mechanism works. * Simplify, fix some issues, and debug instead of run. * Turn on actual testing. * Lower parallelization since GitHub started cancelling tasks. * Try 15 parallel jobs instead. * Turn off fail-fast instead of limiting parallelization. Fail fast seems to be the reason why the tests aren't completing, not quota (since if too many jobs are started, the extras should just be queued until resources open up). * Simplify workflow & allow it to be required. Also, introduce bazelrc file to simplify the CI CLIs interacting with Bazel. * Add test change to investigate computing affected targets. * Another test change to compute affected targets. * Update workflow to use future script compute_affected_tests.sh. Also, ignore Bazel directories in Git (to ease local development). * Add script to compute affected targets. This script is primarily meant to be used for CI. * Execute tests in parallel to build. This creates a new job to compute affected targets alongside the build. This may result in the initial build being a bit slower, but subsequent commits should be fast if remote caching is enabled. This will also result in better performance for forks that can't use remote caching. * Script clean-ups. Also, re-trigger actions. * Try to ensure develop branch is available for change analysis. * Add automatic workflow cancellation. Also, add support to explicitly run all tests if '[RunAllTests]' is in the PR title. * Attempt to make conditional matrix computation work. * Remove join since it was used incorrectly. * Add support for testing when Bazel, BUILD, and WORKSPACE files change. One consequence is the current Bazel file structure is very tight, so any changes will likely run the whole suite. This will get better over time. Also, show test logs for all test runs. * Fix broken tests by adding missing dep library. * Finalize PR. 1. Expand codeowners to include all workflow files. 2. Remove test comments in Kotlin files. 3. Re-enable all workflows. 4. Attempt to fix tests broken on actions but not locally by adding more thread synchronization points. * Lint fix. * Fix timing issues and JDK 9-specific regression. See comment thread in #1904 for more context. * Restore workflow names + introduce test check. The new test check workflow will be a static job that will be required to pass. One failing test is being introduced to verify that this check fails as expected. The original workflow names are being restored so that they don't need to be renamed in GitHub settings (since that would introduce a discontinuity in CI service & require multiple migratiaon PRs to fix). * Update StateFragmentLocalTest.kt Remove fail-on-purpose test since it verified what it needed to: the new test status check job is working as expected. * Address reviewer comments. * Fix most tests broken in Bazel after syncing. * Gitignore + fix broken test. The test failure here only happens when using JDK9+ (which doesn't happen in the Gradle world, or on CI). The .gitignore is since we can't yet specify a .bazelproject in a shareable way. * Lint fixes. * Post-merge clean-up. * Fix broken post-merge test. * Remove unnecessary codeowners per earlier codeowners setup. * Fix ItemSelectionInputDoesNotContainAtLeastOneOfRuleClassifierProviderTest. * Disable image region selection tests. These tests can't correctly pass on Robolectric until #1611 is fixed, so disabling them for the time being to avoid the image loading flake happening on CI (but not locally). Note that chances are a fix will still be needed for the flake, but that can be addressed later. * Disable 2 previously missed tests. * Post-merge lint fix. * Add missing dependency. Verified all tests build & pass both for JDK 9 & 11. Hopefully they will work as expected on CI. * Add missing codeowners for new files. * Post-merge fixes. This fixes some tests that were broken after recent PRs, and fixed a visibility error introduced in #2663. * Move Bazel tests to new workflow. This will make it easier to restart failures without having to also restart unrelated tests.
Fix #2659.
This PR introduces a wrapper library for all third party dependencies for the following benefits:
artifact
)This doesn't quite handle all cases (e.g. see WORKSPACE), but it handles most & is at least a step in the right direction.
This PR includes some unrelated fixes in format_import_proto_library.
I'm blocking this PR on #1904 since that actually fixes all test builds Bazel, and I can't be confident this PR is correct until I see those builds succeed & run in CI. I only verified
//:oppia
so far.