Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Remove Windows from CI - #413

Closed
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci
Closed

Remove Windows from CI#413
keithmattix wants to merge 8 commits into
proxy-wasm:mainfrom
keithmattix:remove-wasmtime-windows-from-ci

Conversation

@keithmattix

@keithmattixkeithmattix commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Due to a general lack of ecosystem support for Windows (e.g. Envoy, Bazel) and our inability to get Windows working with newer toolchains, we've decided to remove Windows from CI until the project has active contributors with sufficient expertise to make it work

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

@PiotrSikoraPiotrSikora left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! You could also add WAMR on Windows in this PR and remove Wasmtime on Windows as part of #406, since it still works until the update.

@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Good idea! I'll do that

Comment thread.github/workflows/test.yml Outdated
Comment thread.github/workflows/test.yml Outdated
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

@PiotrSikora

PiotrSikora commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah nope looks like no Windows for WAVM; false alarm. Will close this and I guess we should just merge the other PR

It looks like CMake issue, so it might be worth updating rules_foreign_cc to a more recent version, since the one we use now is from ~2.5 years ago.

If that doesn't automagically fix the build, then I agree it's not worth pursuing further.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
keithmattixforce-pushed the remove-wasmtime-windows-from-ci branch from de48c52 to 4b834dfCompareAugust 24, 2024 14:15
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

@PiotrSikora

Copy link
Copy Markdown
Member

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Running into an issue with rules_foreign_cc that isn't due to the build but rather bazel nested dependencies and ordering. From what I understand, bzlmod is supposed to fix this, but not sure if there's a way around it without bzlmod. In short, the dependencies for rules_foreign_cc need to be pulled in ~immediately after the repository is imported because newer versions depend on bazel_features and a couple of other things. However, because we load our repos in a function, we can't load() the just-imported dependencies inside that function under bazel rules

See bazel/dependencies.bzl and bazel/dependencies_import.bzl.

Ah, I didn't push up my latest changes; I played around with both of those files, but I think there's an import cycle since we pull the wavm repositories and stuff in the same function we import the bazel rules. I suppose I could try to split them up?

keithmattixand others added 4 commits August 25, 2024 21:55
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix

Copy link
Copy Markdown
ContributorAuthor

Ok I figured out the bazel issues, and Linux builds fine, but looks like there's still some pathing issues on Windows

@PiotrSikora

Copy link
Copy Markdown
Member

Linux builds fine, but looks like there's still some pathing issues on Windows

ACK, thanks for trying! Feel free to drop Windows support.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattixkeithmattix changed the title Remove wasmtime windows from CI and test other enginesRemove Windows from CIAug 26, 2024
@PiotrSikora

Copy link
Copy Markdown
Member

As mentioned before, removal of Wasmtime on Windows from the CI should be done as part of #406, since it works fine until the Wasmtime update, so this PR can be closed. Thanks!

Sign up for freeto 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.

2 participants

@keithmattix@PiotrSikora