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

kibana 7.6.2 #55967

Closed
wants to merge 1 commit into from
Closed

kibana 7.6.2 #55967

wants to merge 1 commit into from

Conversation

ankane
Copy link
Contributor

@ankane ankane commented Jun 9, 2020

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

For #55419

xpack is no longer included in package.json (elastic/kibana#32722), but completely removing it from projects.ts is a bit more complicated now: https://github.com/elastic/kibana/blob/c14a620411be7e6e463520eafa61fa8d7efb84ce/src/dev/typescript/projects.ts#L29-L35

Since we pass the --oss flag to build, it seems like we shouldn't need to remove it.

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Can you also try a newer python version and improving the test?

Formula/kibana.rb Show resolved Hide resolved
@ankane
Copy link
Contributor Author

ankane commented Jun 9, 2020

Improved the test. What do you mean by the Python part?

@SMillerDev
Copy link
Member

It currently depends on macOS due to a requirement for python 2. Could 7.x work with python 3?

@ankane
Copy link
Contributor Author

ankane commented Jun 9, 2020

What's the best way to ensure it's not using system Python? I've tried adding a fake python script to the PATH to see if it'd fail, as well as updating ENV["PYTHON"], but the formula still succeeds, which makes me think neither of those options are affecting the build.

@SMillerDev
Copy link
Member

If you make it depend on python@3.8 brew should do the rest.

@ankane
Copy link
Contributor Author

ankane commented Jun 10, 2020

Looks like it succeed, so hopefully that means it'll work outside of Mac.

This was referenced Jun 10, 2020
s.gsub! "new Project(resolve(REPO_ROOT, 'x-pack/tsconfig.json')),", ""
s.gsub! "new Project(resolve(REPO_ROOT, 'x-pack/test/tsconfig.json'), 'x-pack/test'),", ""
end

inreplace "package.json", /"node": "10\.\d+\.\d+"/, %Q("node": "#{Formula["node@10"].version}")
system "yarn", "kbn", "bootstrap"
system "yarn", "build", "--oss", "--release", "--skip-os-packages", "--skip-archives"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably directly call the build strict with

Suggested change
system "yarn", "build", "--oss", "--release", "--skip-os-packages", "--skip-archives"
system "node", "scripts/build", "--oss", "--release", "--skip-os-packages", "--skip-archives"

here, because yarn build calls it with --all-platforms which is unnecessary for us.

depends_on "node@10"

def install
# remove non open source files
rm_rf "x-pack"

This comment was marked as resolved.

@chrmoritz chrmoritz mentioned this pull request Jun 11, 2020
13 tasks
@ankane
Copy link
Contributor Author

ankane commented Jun 11, 2020

Thanks @chrmoritz, looks like both the suggestions worked!

@chrmoritz
Copy link
Contributor

Great! Can you confirm that everything works with the updated *beat formulae together. I'm not sure about how much I want to trust solely on the isolated test inside the formulae.

@chrmoritz chrmoritz mentioned this pull request Jun 11, 2020
13 tasks
@ankane
Copy link
Contributor Author

ankane commented Jun 11, 2020

I'm not very familiar with the *beat tools. What's the main concern there?

@SMillerDev
Copy link
Member

@chrmoritz is this ready to go?

@chrmoritz
Copy link
Contributor

Kinda, if something isn't working correctly, which isn't covered by the isolated test, then people will open issues I guess. I'm also not someone with enough experience with the elastic stack to extensively test it completely. So fingers crossed, but I'm quite confident that it should work.

Also it's still better than further keeping these formulae on the outdated 6.x release line.

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Thanks @ankane ! Without contributions like yours it'd be impossible to keep homebrew going with the high standards that users have come to expect from the project. You can feel good knowing that you've made the world a tiny bit better for homebrew users around the world! 👍 🎉

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

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.

4 participants