Skip to content

factor out config.sh script to update config files (VERSION, SYSCONFDIR)#7036

Merged
dlang-bot merged 1 commit intodlang:masterfrom
MartinNowak:fixup_dub_package
Jul 31, 2017
Merged

factor out config.sh script to update config files (VERSION, SYSCONFDIR)#7036
dlang-bot merged 1 commit intodlang:masterfrom
MartinNowak:fixup_dub_package

Conversation

@MartinNowak
Copy link
Member

  • use generated/dub as string import path for copied/generated config files

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @MartinNowak!

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.

@MartinNowak
Copy link
Member Author

FYI @jacob-carlborg

@wilzbach
Copy link
Contributor

Could we first focus on re-enabling the Travis test?
WIP: #6999

// https://github.com/dlang/dub/issues/1199
dflags "-J$PACKAGE_DIR"
preGenerateCommands "\"$${DUB_PACKAGE_DIR}config.sh\" \"$${DUB_PACKAGE_DIR}generated/dub\" VERSION /etc"
stringImportPaths "generated/dub"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work when building a package that has DMD as a dependency? I had problems with this before, that's why I switch from -J. to -J$PACKAGE_DIR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well there is a difference between passing raw paths directly to dmd and telling dub which subfolder to use in a package. And yes it will work since dub knows how to deal with relative paths for the latter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, ok. Using . does not work (at least not in this project). But other paths seem work.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, . does exactly what you want.
I'm seeing a small issues with non-absolute arguments to dub --root=dmd build :lexer where dub tries to relate two relative paths.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

No, . does exactly what you want.

I don't know how, but I got this error Invalid variable: DFLAGS when using a dot .. As you can see in dlang/dub#1199. For some reason I don't get this error anymore today 😕.

dub.sdl Outdated
// stringImportPaths cannot be used because it will make Dub extremely slow
// https://github.com/dlang/dub/issues/1199
dflags "-J$PACKAGE_DIR"
preGenerateCommands "\"$${DUB_PACKAGE_DIR}config.sh\" \"$${DUB_PACKAGE_DIR}generated/dub\" VERSION /etc"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use backticks instead of quotes to avoid the need to escape the quotes.

dub.sdl Outdated
// stringImportPaths cannot be used because it will make Dub extremely slow
// https://github.com/dlang/dub/issues/1199
dflags "-J$PACKAGE_DIR"
preGenerateCommands "\"$${DUB_PACKAGE_DIR}config.sh\" \"$${DUB_PACKAGE_DIR}generated/dub\" VERSION /etc"
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, you're using one of the few undocumented environment variables 😃.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not documented, but quite accessible to anyone invoking env and using git grep (or just searching the project).
https://github.com/dlang/dub/blob/dafdc47d690c63d074e1b9ed57349fe2eaaabd2b/source/dub/generators/generator.d#L480-L519

Copy link
Member Author

Choose a reason for hiding this comment

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

@MartinNowak MartinNowak force-pushed the fixup_dub_package branch 2 times, most recently from 13674bc to ab4a410 Compare July 26, 2017 19:10
@MartinNowak
Copy link
Member Author

Could we first focus on re-enabling the Travis test?
WIP: #6999

It's somewhat unrelated and #6999 isn't quite ready yet.

config.sh Outdated
@@ -0,0 +1,15 @@
#/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the bang !.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting that it still worked when being invoked from make and dub.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but not from the shell. I got some really cryptic error message.

@@ -0,0 +1,15 @@
#/bin/sh

Copy link
Contributor

Choose a reason for hiding this comment

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

Add some error handling, like set -e.

Copy link
Member Author

Choose a reason for hiding this comment

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

- use generated/dub as string import path for copied/generated config files
@jacob-carlborg
Copy link
Contributor

LGTM 👌.

@dlang-bot dlang-bot merged commit 2fbbcd9 into dlang:master Jul 31, 2017
@MartinNowak MartinNowak deleted the fixup_dub_package branch August 30, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments