Skip to content

azure: add OMF build with LATEST dmd#10253

Merged
wilzbach merged 2 commits intodlang:masterfrom
rainers:azure_omf
Aug 3, 2019
Merged

azure: add OMF build with LATEST dmd#10253
wilzbach merged 2 commits intodlang:masterfrom
rainers:azure_omf

Conversation

@rainers
Copy link
Member

@rainers rainers commented Jul 31, 2019

Also:

  • do not download some files if already existing (for local testing of the script)
  • simplify space handling

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @rainers!

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.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10253"

@rainers rainers mentioned this pull request Jul 31, 2019
Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Generally LGTM 👍

else
export MODEL_FLAG="-m32"
MAKE_FILE="win32.mak"
LIBNAME=phobos.lib
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LIBNAME=phobos.lib
LIBNAME=phobos32.lib

Copy link
Member Author

Choose a reason for hiding this comment

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

phobos.lib is the name of the OMF library so far. That's why I had to special case it.

@rainers
Copy link
Member Author

rainers commented Jul 31, 2019

Do we have more free jobs on Azure? Testing a debug build would be nice, too.

@wilzbach
Copy link
Contributor

Yes we have 10x parallelism (:

@PetarKirov
Copy link
Member

Open-source projects have unlimited free jobs and up to 10 of them can execute in parallel ;)

@rainers
Copy link
Member Author

rainers commented Aug 1, 2019

Open-source projects have unlimited free jobs and up to 10 of them can execute in parallel ;)

Thanks, good to know.

I've restored the quotes and repaired the 64-bit build (it didn't do anything after my change).

@marler8997
Copy link
Contributor

To address the failure on azure, you'll probably need to add quotes around the compiler string when building the command string in d_do_test.d line 467:

< string command = compiler;
> string command = `"` ~ compiler ~ `"`;

@rainers
Copy link
Member Author

rainers commented Aug 1, 2019

To address the failure on azure, you'll probably need to add quotes around the compiler string when building the command string in d_do_test.d line 467:

Yeah, noticed that, too. Added now while trying to make sure no double quoting happens.

/// add quotes around the whole string if it contains spaces that are not in quotes
string quoteSpaces(string str)
{
if (str.indexOf(' ') < 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this smaller version work?

if (str.startsWith(`"`) || str.indexOf(' ') < 0)
    return str;
return `"` ~ str ~ `"`;

Copy link
Member Author

Choose a reason for hiding this comment

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

Not with the current default:

envData.ccompiler = `C:\"Program Files (x86)"\"Microsoft Visual Studio 10.0"\VC\bin\amd64\cl.exe`;

Copy link
Contributor

@marler8997 marler8997 Aug 1, 2019

Choose a reason for hiding this comment

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

Jesus BATCH is friggin weird

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want it to also handles spaces on posix systems, you'll probably need a different implementation for posix. But not a big deal since there's probably alot of other stuff that wouldn't work with the DMD build if there were spaces in the path.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you want it to also handles spaces on posix systems, you'll probably need a different implementation for posix. But not a big deal since there's probably alot of other stuff that wouldn't work with the DMD build if there were spaces in the path.

I also wouldn't expect spaces in filenames on posix to be battle tested. It's also quite a mess dealing with them between bash, make and every other tool.

@wilzbach wilzbach merged commit 04f0e3f into dlang:master Aug 3, 2019
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.

5 participants

Comments