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

Build OpenSearch on Windows #767

Merged
merged 5 commits into from
Oct 22, 2021
Merged

Conversation

dblock
Copy link
Member

@dblock dblock commented Oct 19, 2021

Description

Fixes tests on Windows:

  1. Use os.path.join vs. concatenating string paths or paths with a / separator.
  2. Git on windows writes read-only files that cannot be unlinked, use workaround in https://stackoverflow.com/questions/1213706/what-user-do-python-scripts-run-as-in-windows.
  3. Fixup load paths in tests/tests_build_workflow.
  4. Make OpenSearch-min build script work on Windows.

Issues Resolved

On top of #772.

Part of #33.

With this PR and fixes in opensearch-project/OpenSearch#1412 you can successfully build OpenSearch 2.0.0 min with the following manifest.

build:
  name: OpenSearch
  version: 2.0.0
components:
  - name: OpenSearch
    repository: https://github.com/dblock/OpenSearch.git
    ref: "fix-windows-build"
    checks:
      - gradle:publish
      - gradle:properties:version
schema-version: '1.0'
$ ./build.sh manifests/2.0.0/opensearch-2.0.0.yml --snapshot

$ find artifacts/dist/
artifacts/dist/
artifacts/dist/opensearch-min-2.0.0-SNAPSHOT-windows-x64.zip

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@peternied
Copy link
Member

Look at this! I'll review tomorrow morning

@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2021

Codecov Report

Merging #767 (a308d6d) into main (60345d6) will decrease coverage by 0.32%.
The diff coverage is 75.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
- Coverage   91.30%   90.98%   -0.33%     
==========================================
  Files          75       75              
  Lines        1990     2018      +28     
==========================================
+ Hits         1817     1836      +19     
- Misses        173      182       +9     
Impacted Files Coverage Δ
src/build_workflow/builder.py 100.00% <ø> (ø)
src/manifests/build_manifest.py 100.00% <ø> (ø)
src/manifests/bundle_manifest.py 100.00% <ø> (ø)
src/run_perf_test.py 0.00% <0.00%> (ø)
src/test_workflow/test_args.py 100.00% <ø> (ø)
src/test_workflow/integ_test/local_test_cluster.py 80.83% <20.00%> (-6.24%) ⬇️
src/run_ci.py 82.35% <66.66%> (ø)
src/run_build.py 85.00% <70.00%> (ø)
src/system/temporary_directory.py 95.65% <95.00%> (-4.35%) ⬇️
src/assemble_workflow/bundle.py 97.14% <100.00%> (ø)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60345d6...a308d6d. Read the comment docs.

@dblock dblock changed the title Fix windows tests Fix OpenSearch-min build on windows and tests Oct 20, 2021
@dblock dblock changed the title Fix OpenSearch-min build on windows and tests Windows support Oct 20, 2021
@dblock dblock changed the title Windows support Build OpenSearch on Windows Oct 20, 2021
@@ -48,7 +49,7 @@ def __init__(self):
self.snapshot = args.snapshot
self.component = args.component
self.keep = args.keep
self.script_path = sys.argv[0].replace("/src/run_build.py", "/build.sh")
self.script_path = sys.argv[0].replace(os.path.sep + os.path.join("src", "run_build.py"), f"{os.path.sep}build.sh")
Copy link
Member

@gaiksaya gaiksaya Oct 21, 2021

Choose a reason for hiding this comment

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

Interesting os.path.sep

Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this :shipit:

dblock and others added 3 commits October 21, 2021 23:16
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@dblock.org>
Signed-off-by: dblock <dblock@dblock.org>
Signed-off-by: dblock <dblock@dblock.org>
Copy link
Contributor

@VachaShah VachaShah left a comment

Choose a reason for hiding this comment

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

LGTM!

@dblock dblock merged commit 7f3e41f into opensearch-project:main Oct 22, 2021
@dblock dblock deleted the fix-windows branch October 22, 2021 00:47
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.

None yet

5 participants