-
Notifications
You must be signed in to change notification settings - Fork 611
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
please.sh: add asciidoctor to build-installers #572
please.sh: add asciidoctor to build-installers #572
Conversation
please.sh
Outdated
# Asciidoctor (requires Ruby to run) | ||
$PREFIX/bin/asciidoctor | ||
$PREFIX/bin/asciidoctor.bat | ||
$PREFIX/lib/ruby/gems/*/gems/asciidoctor-*/ | ||
$PREFIX/bin/ruby.exe | ||
$PREFIX/bin/ruby*.dll |
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.
Looking at the package details, I think this should do the job. Curious to hear if you have any thoughts or concerns about this.
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.
Was able to build build-installers
locally. Looks like it's still missing a few things:
$ asciidoctor --version
`RubyGems' were not loaded.
`error_highlight' was not loaded.
`did_you_mean' was not loaded.
C:/repos/build-installers/clangarm64/bin/asciidoctor:16:in `require': cannot load such file -- rubygems (LoadError)
from C:/repos/build-installers/clangarm64/bin/asciidoctor:16:in `<main>'
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.
Looks like I need to add $PREFIX/lib/ruby/
(the entire folder) instead of just $PREFIX/lib/ruby/gems/*/gems/asciidoctor-*/
, so I've just force-pushed to make that change. Would that be OK for you?
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.
Would you happen to know the difference of the size of the build-installers
artifact before and after this patch? I am only a bit concerned about making the artifact unnecessarily large.
In an earlier commit, Git for Windows switched from its own *-asciidoctor-extensions package to the MSYS2-provided *-asciidoctor package. However, ARM64 pipelines that use the build-artifacts SDK flavor now complain that the asciidoctor binary isn't available. This commit should ensure that Asciidoctor becomes available. Ref: git-for-windows@eb16e99 Signed-off-by: Dennis Ameling <[email protected]>
02cb5f2
to
c713493
Compare
Hmm. I took a deep look at why this is needed at all, when it seems to work all right for the i686 and x86_64 builds. And I think I have the answer. In git-for-windows/git-sdk-arm64#6, we replaced the AsciiDoctor paths in However, in the meantime AsciiDoctor did become supported, and we even switched over from AsciiDoc to AsciiDoctor (installing the latter in git-for-windows/git-sdk-arm64@cbf9ddf). Which means that the AsciiDoc paths need to be replaced by the AsciiDoctor paths in With these changes in git-for-windows/git-sdk-arm64, I believe we can drop this here PR. |
@dennisameling I have opened git-for-windows/git-sdk-arm64#21 (without testing because I'd have to spin up a VM for that and that takes more time than I have right now), could you give it a whirl? Feel free to force-push as needed. |
@dennisameling now that I verified that the |
Closing in favor of git-for-windows/git-sdk-arm64#21. Thanks so much @dscho! |
This pipeline failed because
asciidoctor
wasn't available. However, it is available in thegit-sdk-arm64
repo. Looks like it just doesn't end up in thebuild-installers
SDK flavor which is used by that pipeline.In an earlier commit, Git for Windows switched from its own *-asciidoctor-extensions package to the MSYS2-provided *-asciidoctor package.
However, ARM64 pipelines that use the build-artifacts SDK flavor now complain that the asciidoctor binary isn't available.
This commit should ensure that Asciidoctor becomes available.
Ref: eb16e99