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.

Sign up for GitHub

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

Jump to bottom

Fix mimirtool build windows #2273

Merged
merged 3 commits into from
Jun 29, 2022
Merged

Fix mimirtool build windows #2273

merged 3 commits into from
Jun 29, 2022

Conversation

pstibrany
Copy link
Member

@pstibrany pstibrany commented Jun 29, 2022 •
edited
Loading

What this PR does

This PR imports windows-related mmap code from Prometheus https://github.com/prometheus/prometheus/blob/main/tsdb/fileutil/ package to make mimirtool buildable on Windows.

Note that target for this bugfix was set to release-2.2.

Which issue(s) this PR fixes or relates to

Fixes #2258

Checklist

  • [na] Tests updated
  • [na] Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pstibrany
Fix mimirtool build on windows.
f7a5cbc 
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
@pstibrany pstibrany changed the base branch from main to release-2.2 June 29, 2022 09:19
@pstibrany
CHANGELOG.md
8ba7d9c 
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
aknuds1
aknuds1 reviewed Jun 29, 2022
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

See question.

pkg/storegateway/indexheader/fileutil/mmap_windows.go Show resolved Hide resolved
@aknuds1 aknuds1 requested a review from a team June 29, 2022 09:31
aknuds1
aknuds1 approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM

aknuds1
aknuds1 requested changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Realized the changelog entry should be moved.

CHANGELOG.md Outdated Show resolved Hide resolved
@pstibrany
Move changelog entry to 2.2.0-rc.1 / Mimirtool section.
35f7695 
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
stevesg
stevesg approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@stevesg stevesg left a comment

Choose a reason for hiding this comment

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

Question: Where do we document what platforms we support for Mimir vs Mimir Tool?

Context: I left Windows out of this code originally because I was not under the impression we supported Windows.

@pstibrany
Copy link
Member Author

Question: Where do we document what platforms we support for Mimir vs Mimir Tool?

@pracucci mentions that we have committed to release mimirtool for Windows in #1763 (reply in thread). Plus our dist target in the Makefile tries to build Windows binaries for mimirtool, but nothing else.

aknuds1
aknuds1 approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM

@pstibrany pstibrany merged commit 950ce5c into release-2.2 Jun 29, 2022
@pstibrany pstibrany deleted the fix-mimirtool-build-windows branch June 29, 2022 13:25
@pstibrany pstibrany mentioned this pull request Jun 30, 2022
Closed
7 tasks
masonmei pushed a commit to udmire/mimir that referenced this pull request Jul 25, 2022
@pstibrany
Fix mimirtool build windows (grafana#2273)
c12e2e8 
* Fix mimirtool build on windows.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

* CHANGELOG.md

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

* Move changelog entry to 2.2.0-rc.1 / Mimirtool section.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknuds1 aknuds1 aknuds1 approved these changes

@stevesg stevesg stevesg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mimirtool fails to build on Windows
3 participants
@pstibrany @aknuds1 @stevesg