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

Implemented feature 767 (ready for review & pull) #1839

Merged
merged 8 commits into from Jun 9, 2013

Conversation

eskimor
Copy link

@eskimor eskimor commented Apr 4, 2013

http://d.puremagic.com/issues/show_bug.cgi?id=767

Implemented -deps switch without an '=file' it prints dependencies to stdout. For module imports the format is the same as for -deps=file, but the line is prefixed with depsImport.

The following dependencies are printed:
depsVersion Any version() found (except standard ones and ones set in the module itself)
depsImport Any imports found (same as -deps=file output, except prefixed with depsImport)
depsFile Any file imports found
depsLib Any libraries specified with pragma(lib)
depsDebug Any debug() statements found (except the ones set in the module itself)

The format is extensible, as additional dependencies can be easily added by providing some new prefix depsFOO. Tools can easily grasp what is dependency information, because every line starts with "deps".

This patch is needed for tools like dub for reliable and efficient builds.

@eskimor
Copy link
Author

eskimor commented Apr 7, 2013

Based on feedback on IRC: Dropped -filedeps, instead recycled -deps. If no file is specified, print to stdout in the format explained in the commit message. (Compatible to -v output)

issue 767 is now fully implemented and a bit more.

@eskimor
Copy link
Author

eskimor commented Apr 19, 2013

Any feedback? If you as a maintainer don't like it, please just tell me. Knowing that it won't be accepted is still better than waiting forever. If it looks good to you but you haven't found the time yet to proof read it, please also just leave a short comment.

I am also very willing to improve this patch in any possible way, just tell me.

I would really like to improve dub based on this feature, but I need to know whether it is going to be accepted or not.

@nazriel
Copy link
Contributor

nazriel commented Apr 19, 2013

@eskimor , this is very good work.
You need to be patience though.
Until recently, some pull requests were hanging for even a year, before they were merged.
Situation is better now, but still takes some time ;)

@eskimor
Copy link
Author

eskimor commented Apr 19, 2013

@eskimor , this is very good work.

:-) Thanks a lot!

You need to be patience though.
Until recently, some pull requests were hanging for even a year, before they were merged.

That's exactly what scared me :-) I guess I'll start with some dub improvements, that do not depend on this.

Thanks again for your feedback, I really appreciate it.

(Only string imports are tracked, no pragma(lib)).
Purpose: Output everything dependency related to stdout.

Prints to stdout + prefix for different kind of dependencies:

depsVersion Any version() found
depsImport Any imports found (same as -deps=file output, except prefixed with depsImport)
depsFile Any file imports found
depsLib Any libraries specified with pragma(lib)
depsDebug  Andy debug() statements found (Not yet implemented)
depsDebug are currently printed for every occurrence of debug(something)
-> have to find out why.
{
char *name = (char *)mem.malloc(se->len + 1);
memcpy(name, se->string, se->len);
name[se->len] = 0;
printf("library %s\n", name);
Copy link
Member

Choose a reason for hiding this comment

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

why the extra whitespace?

@andralex
Copy link
Member

andralex commented Jun 4, 2013

there should be sample flags usage and output in the comments

@timotheecour
Copy link
Contributor

Why not use json output instead of a custom one

(Every active import from a module will be printed like this: "bldinfo import: %s("%s") => %s("%s")", moduleFQN, moduleFileName,importedModuleFQN, importedModuleFileName)

It'll make it easier to parse

@dnadlinger
Copy link
Member

Apart from the points brought up above, this needs test cases!

@eskimor
Copy link
Author

eskimor commented Jun 5, 2013

I hope I got all whitespace issues. I'll take care of explanatory comments asap.
@timotheecour I kept the format which was used in -deps=filename, thought this might be good. Not sure if JSON really helps for something that simple.
@klickverbot Ok, I'll check out the testing infrastructure and fiddle something together.

Thank you guys!

@dnadlinger
Copy link
Member

@eskimor: Yep, I'd just check out some of the existing test cases that validate the compiler output (DDoc, JSON, …) and adapt it for this purpose.

@eskimor
Copy link
Author

eskimor commented Jun 6, 2013

@timotheecour Also the output is to stdout, so the Json would be mixed into other output.

@eskimor
Copy link
Author

eskimor commented Jun 6, 2013

Thanks @klickverbot for the hint :-) Before I forget again, I needed the helper routine 'escapePath()' in more than one place, so I simply moved it into mars.h/c. Is this the right place for general helper functions or is there a better one?

WalterBright added a commit that referenced this pull request Jun 9, 2013
Implemented feature 767 (ready for review & pull)
@WalterBright WalterBright merged commit 2f33a0e into dlang:master Jun 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants