Adapt to generic Makefiles #565

kinke · 2023-12-17T06:00:10Z

Depends on

Soft depends on

Fix Issue 22212 - dmd version has -dirty suffix on windows #566

The Windows VM used for the releases needs a GNU make; mingw32-make.exe is available for CI (the first make.exe in PATH there is DigitalMars make), so I went with that for now. FWIW, I've just seen that GNU make can also be installed via Chocolatey (choco install make).

dlang-bot · 2023-12-17T06:00:13Z

Thanks for your pull request and interest in making D better, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Don't build the 32-bit COFF libraries as extras of the 64-bit build. Instead, build them as the default 32-bit libraries that they are, and build the OMF libraries as extras of the 32-bit build.

And get rid of superfluous manual object-files cleanup.

ibuclaw · 2023-12-26T09:39:45Z

So, without this patch, master fails to build on windows with:

Error: can't read makefile 'win32. mak'

ibuclaw · 2023-12-26T09:55:12Z

With this patch:

'mingw-make is not recognised as an internal or external command,
operable program or batch file.

ibuclaw · 2023-12-26T12:17:35Z

Searching for anything that looks like a make command inside the windows release box, there's only

dm/bin/make
VC/Tools/MSVC/.../bscmake
VC/Tools/MSVC/.../nmake
VC/Tools/MSVC/.../xcdmake

ibuclaw · 2023-12-26T12:35:48Z

The mingw project on sourceforge has prebuilt binaries for mingw32-make. Fumbling around seeing if I can build a derivative vagrant box with just that file in the path added.

ibuclaw · 2023-12-26T20:56:03Z

The mingw project on sourceforge has prebuilt binaries for mingw32-make.

That failed with

makefile:32: Extraneous text after `else' directive
makefile:34: Extraneous text after `else' directive
makefile:34: *** only one `else' per conditional.  Stop.

The version that worked inside the box was 3.80. Looking at the makefile, I see there's a download link to 4.4 on the dmd github releases page, duh.

ibuclaw · 2023-12-26T21:49:57Z

Alright, looks like installing gnumake-4.4 inside the box is doing the trick.
Only (probably harmless) warning I see is

Using make SHELL "C:/Program Files/Git/usr/bin/bash.exe", should be bash.

Another I saw when it moves to build dlang/tools is

'[' is not recognised as an internal or external command,
operable program or batch file.

Doesn't break the build, but it was introduced by dlang/tools#465

However, the last step (Copy Sources) fails on Windows at the following location (new comment thread)....

ibuclaw · 2023-12-26T21:55:58Z

create_dmd_release/create_dmd_release.d

+    static void ensureIsClean(string repoDir)
+    {
+        const output = runCapture("cd "~quote(repoDir)~" && git status --porcelain");
+        if (output.length)
+            fail("Repo '"~repoDir~"' is dirty:\n" ~ output);
+    }
+
    // Copy sources
+    ensureIsClean(cloneDir~"/dmd");
+    ensureIsClean(cloneDir~"/phobos");


... Here.

@schveiguy mentioned this last year, but also again recently in a DLF call. For some reason dmd releases have always had -dirty in the version string. No surprises then that this fails.

https://gist.github.com/ibuclaw/970f0722c20a5979b48cfc9fa38ded0c

The list appears to be arbitrary, so I suspect it's some automated unix2dos conversion (git itself is one known culprit for doing this).

Running the commands individually inside the box, it differs at the first hurdle:

old mode 100755 new mode 100644

This is every file in the list, so it would appear scp command is at fault for not preserving permissions.

https://github.com/dlang/installer/blob/master/create_dmd_release%2Fbuild_all.d#L153-L159

Instead invoking scp -r - p I get either of the following for various files.

scp: ... : set times: Input/output error scp: ... : set times: Broken pipe

https://github.com/dlang/installer/blob/master/create_dmd_release%2Fcreate_dmd_release.d#L297-L308

The best I can think of is to add

version (Windows) run("git config core.fileMode false");

After git clean for all repos in that function.

FAOD, build finally succeeds with suggested filemode change.

Is the version(Windows) necessary?

If permissions of a file change when building on a proper filesystem, we want the build to fail.

kinke · 2023-12-27T13:34:52Z

Thx for testing/verifying/fixing in the release VM.

Alright, looks like installing gnumake-4.4 inside the box is doing the trick. Only (probably harmless) warning I see is
Using make SHELL "C:/Program Files/Git/usr/bin/bash.exe", should be bash.

This is no warning, just some info output I've added in https://github.com/dlang/dmd/blob/4712896a98f7d1182a14afaa6fbd83621667bded/compiler/src/osmodel.mak#L69.

Another I saw when it moves to build dlang/tools is
'[' is not recognised as an internal or external command,
operable program or batch file.
Doesn't break the build, but it was introduced by dlang/tools#465

This is almost certainly https://github.com/dlang/tools/blob/6670ee433f89a120866e3f0a5a3a551f785e7d0d/Makefile#L13; the problem is that we need DMD's osmodel.mak, which also sets up the shell on Windows so that [.exe would be found in PATH, but for that we need to make sure we have a DMD clone first, and that's what that line does. For dlang/tools CI, this works as-is; [.exe is most likely in PATH already (bundled with Git in C:\Program Files\Git\usr\bin). - But as long as it's just a non-fatal error during the build, with an existing ..\dmd clone...

What prevents us from moving the release package generation to CI instead of doing it in some AFAICT loosely-defined VM? The existing GHA workflow seems to cover everything; is it just for secrets needed for codesigning on Windows etc.?

ibuclaw · 2023-12-27T14:15:59Z

'[' is not recognised as an internal or external command,
operable program or batch file.
Doesn't break the build, but it was introduced by dlang/tools#465
This is almost certainly https://github.com/dlang/tools/blob/6670ee433f89a120866e3f0a5a3a551f785e7d0d/Makefile#L13; the problem is that we need DMD's osmodel.mak, which also sets up the shell on Windows so that [.exe would be found in PATH, but for that we need to make sure we have a DMD clone first, and that's what that line does. For dlang/tools CI, this works as-is; [.exe is most likely in PATH already (bundled with Git in C:\Program Files\Git\usr\bin). - But as long as it's just a non-fatal error during the build, with an existing ..\dmd clone...

I think you can use wildcard as a platform agnostic (if a little non-intuitive) way to determine if a directory exists in GNU make.

What prevents us from moving the release package generation to CI instead of doing it in some AFAICT loosely-defined VM? The existing GHA workflow seems to cover everything; is it just for secrets needed for codesigning on Windows etc.?

All build steps are here

https://gist.github.com/ibuclaw/35d19c5ff64ecea25e4a7c7ce6a79eac#file-release-sh-L53https://gist.github.com/ibuclaw/35d19c5ff64ecea25e4a7c7ce6a79eac#file-release-sh-L53

Off the top of my head, at mininum there are

b2/aws secrets for uploading to download dlang.org
ssh access and permissions to upload to ftp.digitalmars.com
commit access to dmd, dub, dlang.org, plus ability to push signed tags to these and phobos, tools, and installer
gpg key for signing tags, packages, tarballs, ...

I don't understand how codesigning is meant to work in an automated context, it all looks 5x more expensive compared to 5-6 years ago, and now it's impossible to do it without a hardware token anyway (or pay even more for cloud signing).

kinke · 2023-12-27T15:41:22Z

Off the top of my head, at mininum there are

b2/aws secrets for uploading to download dlang.org

ssh access and permissions to upload to ftp.digitalmars.com

commit access to dmd, dub, dlang.org, plus ability to push signed tags to these and phobos, tools, and installer

gpg key for signing tags, packages, tarballs, ...

I don't understand how codesigning is meant to work in an automated context, it all looks 5x more expensive compared to 5-6 years ago, and now it's impossible to do it without a hardware token anyway (or pay even more for cloud signing).

Maybe we can split this process up, e.g., via a dedicated CI runner under our control featuring the required secrets. So that it could trigger the release tagging first, then let regular GHA generate the 'raw' install packages, and then post-process (codesigning etc.) & upload these packages on the extra CI runner again. This way, the extra CI runner would probably have way less prerequisites regarding the software stack, not needing the full build toolchain as the current VM(s), which is duplicated for the existing GHA workflows anyway.

ibuclaw · 2023-12-30T17:43:12Z

Maybe we can split this process up, e.g., via a dedicated CI runner under our control featuring the required secrets. So that it could trigger the release tagging first, then let regular GHA generate the 'raw' install packages, and then post-process (codesigning etc.) & upload these packages on the extra CI runner again. This way, the extra CI runner would probably have way less prerequisites regarding the software stack, not needing the full build toolchain as the current VM(s), which is duplicated for the existing GHA workflows anyway.

Could do. I (and @Geod24) have the servers where secrets can be put.

We can create a dlang-release account on github I guess (unless they have the equivalent of a gitlab bot account) - with its own ssh and gpg keys.

I suspect though that it would be simpler to have all repos merged into one though?

ibuclaw · 2023-12-30T18:17:26Z

Merging, otherwise will not be able to build the next beta.

kinke force-pushed the makefiles branch 5 times, most recently from 6351665 to 62f3378 Compare December 21, 2023 17:06

kinke marked this pull request as ready for review December 21, 2023 17:06

kinke added 4 commits December 21, 2023 18:13

Adapt to generic Makefiles

ed6554f

Restructure Windows build

bd4f90a

Don't build the 32-bit COFF libraries as extras of the 64-bit build. Instead, build them as the default 32-bit libraries that they are, and build the OMF libraries as extras of the 32-bit build.

Make sure included dmd+phobos sources are not dirty at copy-time

a2c926b

And get rid of superfluous manual object-files cleanup.

Windows: Bundle missing 64-bit rdmd/ddemangle/dustmite tools

e659ab8

kinke force-pushed the makefiles branch from 62f3378 to e659ab8 Compare December 21, 2023 17:13

Drop support for dub < v1.20

f68ccc1

thewilsonator approved these changes Dec 21, 2023

View reviewed changes

ibuclaw reviewed Dec 26, 2023

View reviewed changes

ibuclaw merged commit 60b1b33 into master Dec 30, 2023
27 of 29 checks passed

ibuclaw deleted the makefiles branch December 30, 2023 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt to generic Makefiles #565

Adapt to generic Makefiles #565

kinke commented Dec 17, 2023 •

edited by ibuclaw

Loading

dlang-bot commented Dec 17, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw Dec 26, 2023 •

edited

Loading

ibuclaw Dec 26, 2023

ibuclaw Dec 26, 2023

ibuclaw Dec 27, 2023

ibuclaw Dec 27, 2023 •

edited

Loading

maxhaton Dec 27, 2023

ibuclaw Dec 30, 2023

kinke commented Dec 27, 2023 •

edited

Loading

ibuclaw commented Dec 27, 2023

kinke commented Dec 27, 2023

ibuclaw commented Dec 30, 2023

ibuclaw commented Dec 30, 2023

Adapt to generic Makefiles #565

Adapt to generic Makefiles #565

Conversation

kinke commented Dec 17, 2023 • edited by ibuclaw Loading

dlang-bot commented Dec 17, 2023

Bugzilla references

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw commented Dec 26, 2023

ibuclaw Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

ibuclaw Dec 26, 2023

Choose a reason for hiding this comment

ibuclaw Dec 26, 2023

Choose a reason for hiding this comment

ibuclaw Dec 27, 2023

Choose a reason for hiding this comment

ibuclaw Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

maxhaton Dec 27, 2023

Choose a reason for hiding this comment

ibuclaw Dec 30, 2023

Choose a reason for hiding this comment

kinke commented Dec 27, 2023 • edited Loading

ibuclaw commented Dec 27, 2023

kinke commented Dec 27, 2023

ibuclaw commented Dec 30, 2023

ibuclaw commented Dec 30, 2023

kinke commented Dec 17, 2023 •

edited by ibuclaw

Loading

ibuclaw Dec 26, 2023 •

edited

Loading

ibuclaw Dec 27, 2023 •

edited

Loading

kinke commented Dec 27, 2023 •

edited

Loading