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

Compilation for Linux, MacOS, and Windows #17

Closed
wants to merge 38 commits into from

Conversation

kuniss
Copy link
Collaborator

@kuniss kuniss commented Mar 5, 2023

  • applied matrix compilation and testing as suggested by the GitHub action for setting up dlang
  • MacOS compilation and testing worked out of the box, Windows alternative needed some adaptations...
  • adjustments to get internal D compiler invocation for generated compiler to run on Windows
  • enhancements to test helper functions to get tests passed on Windows
  • adjustments to some tests where Windows command line is completely different to Unix-like OS
  • renamed GitHub action build file d.yml to build.yml as later we will add a release.yml file

An example of an successful run may be found at here.

@linkrope, I created a PR to allow you to check style of the D source code adaptations, I did and to take the advantages of the GitHub PR tooling.

kuniss added 8 commits March 5, 2023 21:25
of the DMD compiler imvocation to enable compilation on Windows.
to get tests passed on Windows.
Added Windows specific helper function asSingleLineDosInput
is for inline EAG source code tests under Windows.
as Windows is very different here to Linux and MacOS and
any Unix-kind OS.
as later we want to add a "release.yml".
as already contained in example tests.
@kuniss
Copy link
Collaborator Author

kuniss commented Dec 23, 2023

@linkrope Merry Christmas! 😄
Did you see this PR?
I would add to it the executable publication as asset.
Probably I will use https://github.com/marketplace/actions/upload-assets-to-a-release for the publication...

kuniss and others added 11 commits December 27, 2023 19:11
which adds binaries to a published GitHub release.
as "latest" fails. linkrope did it too for dmd.
The versino of ldc was looked up at https://github.com/ldc-developers/ldc/releases matching the version of Frontend, druntime and Phobos to that of dmd.
as "latest" fails. linkrope did it too for dmd.
The versino of ldc was looked up at https://github.com/ldc-developers/ldc/releases matching the version of Frontend, druntime and Phobos to that of dmd.
by ignoring all tags pushes as trigger.
as it is a power shell running by default, not a dos shell.
assuming those are the tags from a release publication. Otherwise, with '*' pattern, build.yml is not triggered at all.
get missed before. Maybe, another order works better...
as excluding version tags let any git push skip the build. Not clear why.
@kuniss
Copy link
Collaborator Author

kuniss commented Dec 27, 2023

Seems, I got the publishing of the 3 different gamma executables working too.
It builds on my fork as visible in this build.
And it gets published to the release page which triggered the build.

The only thing, I did not get managed is to avoid duplicate on tag pushing. It does not work as described at the GitHub Actions documentation...

as in master, see ci.yml.
src/gamma/main.d Outdated Show resolved Hide resolved
src/gamma/main.d Outdated Show resolved Hide resolved
.github/workflows/d.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
kuniss added 9 commits January 3, 2024 22:15
Misused due to a misunderstanding.
based on reusable workflows used at ci.yml and passing built binaries from build job using download-artifact action.
as it all happens on Linux, no Windows runner involved anymore here as in the first workflow version.
into upload.yml to have the same abstractions levels for jobs in release.yml
as it shows an operational error on push. Seems, this token is implicitely inherited.
@kuniss
Copy link
Collaborator Author

kuniss commented Jan 4, 2024

I guess, I found a good solution now. Please review it.

The release binary upload to the release page is now done using the upload/download GH Actions, as it is intended, I guess.
For the result see

I set the uploaded artifact redemption period for a build to 1 day, as it is mainly needed for upload when a release is published. But we could extend it to 90 days max. What do you think?
On a release page the uploaded artifacts should stay forever...

as suggested by @linkrope in linkrope#17 (comment).
Not tested yet on Windows as locally not available, currently. Runs wel on Linux.
src/gamma/main.d Outdated
@@ -250,3 +254,11 @@ void build(string[] fileNames, string outputDirectory)
if (status)
exit(status);
}

string executableName(const string name)
Copy link
Owner

Choose a reason for hiding this comment

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

Make it explicitly private:

private string executableName(const string name)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For my curiosity, why only this method, and not the others in main.d too?
I guess, I "copied" it from the other methods above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done with 49328b8.

src/gamma/main.d Outdated
return name ~ ".exe";
else
return name;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Get rid of this red "minus in circle": in VS Code, enable "insertFinalNewline"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not see the red circle well in my light theme. 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank for the configuration hint regarding "insertFinalNewline"! Didn't know it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done with 49328b8.

@kuniss
Copy link
Collaborator Author

kuniss commented Jan 6, 2024

@linkrope, I just encountered, that dub build is building a debug version:

denis@lisp:~/git/gamma$ dub build
    Starting Performing "debug" build using /usr/bin/dmd for x86_64.

Is my interpretation right?
If so, shouldn't we build a non-debug version when releasing?

@linkrope
Copy link
Owner

linkrope commented Jan 7, 2024

@linkrope, I just encountered, that dub build is building a debug version:

denis@lisp:~/git/gamma$ dub build
    Starting Performing "debug" build using /usr/bin/dmd for x86_64.

Is my interpretation right? If so, shouldn't we build a non-debug version when releasing?

See: https://dub.pm/dub-reference/buildtypes/#default-examples

("optimize", "inline", debugInfo") would be nice: fast code, still with helpful diagnostics.

Note that array bounds check is still enabled in release mode. There is a special build type for release builds with no bounds check. Similiarly, if you believe that contracts (pre -conditions, post-conditstions, invariants) are as valuable as bounds checks, then you should avoid release mode, where contracts are skipped. And use just "optimize" and "inline" instead. The "debug" is not about speed but about size: wouldn't you like to have debug symbols available to understand a backtrace in case of a fatal error?

@kuniss
Copy link
Collaborator Author

kuniss commented Jan 7, 2024

("optimize", "inline", debugInfo") would be nice: fast code, still with helpful diagnostics.

Note that array bounds check is still enabled in release mode. There is a special build type for release builds with no bounds check. Similiarly, if you believe that contracts (pre -conditions, post-conditstions, invariants) are as valuable as bounds checks, then you should avoid release mode, where contracts are skipped. And use just "optimize" and "inline" instead. The "debug" is not about speed but about size: wouldn't you like to have debug symbols available to understand a backtrace in case of a fatal error?

I tend to use --build=release-debug. That way, we would have bound checks and symbol informations in case of exceptions. As far as I understood, we do not have pre-, post-conditions and invariant specifications either. So, we do not need to check them, isn't it?

Or should we define our own build type in dub.json?

    "buildTypes": {
        "release-gamma": {
            "buildOptions": ["debugMode", "optimize", "inline", "debugInfo"]
        }
    }

@kuniss
Copy link
Collaborator Author

kuniss commented Jan 7, 2024

    "buildTypes": {
        "release-gamma": {
            "buildOptions": ["debugMode", "optimize", "inline", "debugInfo"]
        }
    }

Tried this in my PR detached branch and got a Linux binary that is bigger, the other binaries are smaller, see this release page.

I guess, we should run the verification tests with the same build type and LCD compiled to be sure it works, isn't it?

@kuniss
Copy link
Collaborator Author

kuniss commented Jan 7, 2024

I guess, we should run the verification tests with the same build type and LCD compiled to be sure it works, isn't it?

I tried this in this build run on the same branch as above: https://github.com/kuniss/gamma/actions/runs/7439461088
Works!

Now I'm thinking: Shouldn't we use LCD at ci.yml too? I mean, at the end we are interested in checking LCD compiled gamma is working well. That it is working well with DMD, the "developer" should check locally, isn't it?

@linkrope
Copy link
Owner

linkrope commented Jan 8, 2024

Now I'm thinking: Shouldn't we use LCD at ci.yml too? I mean, at the end we are interested in checking LCD compiled gamma is working well. That it is working well with DMD, the "developer" should check locally, isn't it?

This has to wait until "dmd" is no longer hard-coded in main.d? Otherwise, it could be tricky to install both compilers and use ldc to run the test, while dmd is used to compile the generated compiler.

kuniss added a commit to kuniss/gamma that referenced this pull request Apr 11, 2024
as suggested by @linkrope in linkrope#17 (comment).
Not tested yet on Windows as locally not available, currently. Runs wel on Linux.
kuniss added a commit to kuniss/gamma that referenced this pull request Apr 13, 2024
@kuniss
Copy link
Collaborator Author

kuniss commented Apr 13, 2024

I'm going to close this PR as it was substituted by PR #18 which has in general the same content but a newer origin base.

@kuniss kuniss closed this Apr 13, 2024
linkrope pushed a commit that referenced this pull request Jun 18, 2024
* Added Windows specific modifications

of the DMD compiler imvocation to enable compilation on Windows.

* Added executableName() for Windows' ".exe" extension

as suggested by @linkrope and using it build().

* Get rid of OS specific file path separators

as suggested by @linkrope in #17 (comment).

* Enabled matrix build for Linux, MaxOS, and Windows

and the according tests.

* Renamed build.yml to ci.yml

as discussed on PR here: https://github.com/linkrope/gamma/pull/17/files/11c93a9229e028f778573de87b4f478dda96f508#r1437757527

* Refactored out reusable workflows

from ci.yml, to refernce them later there and in release.yml

* Added uploading of built executables

* Avoid CI workflow is running on release publications

and within twice, once for the tag pushed for release and second for the release event.
This was found in https://stackoverflow.com/questions/70743715/how-do-i-configure-a-github-actions-workflow-so-it-does-not-run-on-a-tag-push

* Improved matrix names in GH Actions GUI

* Refactored out upload workflow

into upload.yml to have the same abstractions levels for jobs in release.yml

* Attempt to optimize the built artifacts using a specific build type.

* Switched to single workflow in build.yml

for CI and release builds.

* Get rid of DMD compiler at all for build.

gamma now supports to use any $DC defined D compiler for target compiler compiling.
@kuniss kuniss deleted the cross-compilation branch July 10, 2024 21:39
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.

2 participants