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

DEP1: Implement SDL package description support #582

Merged
merged 8 commits into from
Jun 16, 2015
Merged

Conversation

s-ludwig
Copy link
Member

@s-ludwig s-ludwig commented Jun 9, 2015

This implements DEP1 with a few fields added that are still absent from the wiki page.

@s-ludwig
Copy link
Member Author

s-ludwig commented Jun 9, 2015

Wiki page is updated now.

@s-ludwig
Copy link
Member Author

Since this is an additive change and all tests pass, I'd just merge this as it is. Does anyone want to have a look first? The actual change set is not as big as it looks. The "sdlang-d" commit can just be ignored for the review.

It would also be interesting to review the SDL based format to see if there is anything that could be optimized in terms of usability (see the DEP link at the top).

@grogancolin
Copy link
Contributor

Just testing it out on my local branch now.
I've rewritten my local dubs dub.json to dub.sdl to see if it can parse it.

name "dub"
description "Package manager for D packages"
license "MIT"
copyright "Copyright © 2012-2014 rejectedsoftware e.K., Copyright © 2012-2014 Matthias Dondorff"
authors {
        "Matthias Dondorff" 
        "Sönke Ludwig"
}
targetPath "bin"

configuration {
            name "application"
            targetType "executable"
            mainSourceFile "source/app.d"
            libs "curl"
            versions "DubUseCurl"
}
configuration {
            name "library"
            targetType "library"
            excludedSourceFiles "source/app.d"
            libs "curl"
            copyFiles-windows "bin/libcurl.dll" "bin/libeay32.dll" "bin/ssleay32.dll"
            versions "DubUseCurl"
}
configuration {
            name "library-nonet"
            targetType "library"
            dependency "vibe-d" version="~>0.7.19-rc.4" optional="true"
            excludedSourceFiles "source/app.d"
}

It fails to parse it, throwing an unhelpful error:

./bin/dub                                                                                                                              
Failed to parse package description for dub  in /home/colin/Projects/dub/.
core.exception.RangeError@source/dub/recipe/sdl.d(170): Range violation

It would be good to add a some debug info here for the user to understand whats going wrong.

Also, this is my first time writing an SDL file, so the above may be wrong!
The above SDL is indeed wrong!

Here is the dub.sdl that will build dub:

name "dub"
description "Package manager for D packages"
license "MIT"
copyright "Copyright © 2012-2014 rejectedsoftware e.K., Copyright © 2012-2014 Matthias Dondorff"
authors {
        "Matthias Dondorff" 
        "Sönke Ludwig"
}
targetPath "bin"

configuration "application" {
            targetType "executable"
            mainSourceFile "source/app.d"
            libs "curl"
            versions "DubUseCurl"
}
configuration "library" {
            targetType "library"
            excludedSourceFiles "source/app.d"
            libs "curl"
            copyFiles-windows "bin/libcurl.dll" "bin/libeay32.dll" "bin/ssleay32.dll"
            versions "DubUseCurl"
}
configuration "library-nonet" {
            targetType "library"
            dependency "vibe-d" version="~>0.7.19-rc.4" optional=true
            excludedSourceFiles "source/app.d"
}

My suggestion though would be to add an error something along the lines of:
Failed parsing at

@s-ludwig
Copy link
Member Author

The error checking code was still missing. Now outputs something along the lines of:

Failed to load package in .../: .../dub.sdl(20): Error: Missing value for 'configuration'.

@grogancolin
Copy link
Contributor

Yeah, it's much better/easier to use when the error checking is in there!

Looks good!

@s-ludwig
Copy link
Member Author

Merging to master. @grogancolin Thanks for the review!

s-ludwig added a commit that referenced this pull request Jun 16, 2015
DEP1: Implement SDL package description support
@s-ludwig s-ludwig merged commit 9240fd5 into master Jun 16, 2015
@s-ludwig s-ludwig deleted the dep1-sdl-support branch June 16, 2015 10:41
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.

None yet

2 participants