Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

[reg] fix Issue 14750 - druntime/test/coverage was added to druntime, but n… #1323

Merged
merged 1 commit into from Jul 12, 2015

Conversation

WalterBright
Copy link
Member

…ot to the MANIFEST - zip file broken again

https://issues.dlang.org/show_bug.cgi?id=14750

@DmitryOlshansky
Copy link
Member

Anything we can do to prevent this? Add zip packaging tests to auto-tester?

@rainers
Copy link
Member

rainers commented Jul 10, 2015

We could remove MANIFEST and just use the output of git ls-tree --name-only -r HEAD instead.

@DmitryOlshansky
Copy link
Member

We could remove MANIFEST and just use the output of git ls-tree --name-only -r HEAD instead.

Yeah, would you go for a pull request or should I?

@@ -222,9 +222,13 @@ detab:
detab $(MANIFEST)
tolf $(MANIFEST)


gitzip:
Copy link
Member

Choose a reason for hiding this comment

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

Nice

@rainers
Copy link
Member

rainers commented Jul 10, 2015

Yeah, would you go for a pull request or should I?

Please do ;-)

I guess @WalterBright is the only one using the zip file. I've never even heard of a version of zip named zip32. So it's up to him to decide if it works in his environment.

There are also the detab and checkwhitespace targets using MANIFEST, but fortunately they are only in posix.mak, so we won't have to fight DM make for getting the file list into a make variable.

@rainers
Copy link
Member

rainers commented Jul 10, 2015

For the test failures, Makefile should be excluded from tab checks by adding %/Makefile to the filter in CWS_MAKEFILES.

@DmitryOlshansky
Copy link
Member

Please do ;-)

E-hm if only I knew how to make that zip32 accept a file with filenames...

@WalterBright
Copy link
Member Author

The zip file is for systems that do not have git installed on them, so replacing MANIFEST with git commands doesn't work.

@WalterBright
Copy link
Member Author

The failure is on Linux:

test/shared/Makefile:58:    $(QUIET)$(DMD) -fPIC -shared $(DFLAGS) -of$@ $< $(LINKDL)
test/shared/Makefile:61:    rm -rf $(ROOT)
make: *** [checkwhitespace] Error 1

what does that mean?

@DmitryOlshansky
Copy link
Member

@WalterBright these 2 lines in Makefile contain tabs. Just follow Rainer's suggestion and %/Makefile to the filter in posix.mak to ignore makefiles when checking for tabs and trailing spaces.

@WalterBright
Copy link
Member Author

For the test failures, Makefile should be excluded from tab checks by adding %/Makefile to the filter in CWS_MAKEFILES.

Where is CWS_MAKEFILES ? It isn't in druntime.

@rainers
Copy link
Member

rainers commented Jul 12, 2015

Where is CWS_MAKEFILES ? It isn't in druntime.

I suspect you are on an older branch here. The check for whitespaces has been merged a couple of days ago.

@rainers
Copy link
Member

rainers commented Jul 12, 2015

The zip file is for systems that do not have git installed on them, so replacing MANIFEST with git commands doesn't work.

Moving to a system without git should be no problem. To move a zip file back, you could either "freshen" an existing zip (e.g. -f with info-zip) or just run make clean before zipping the whole directory.

@WalterBright
Copy link
Member Author

I never trust freshen, as sometimes files get removed from git. make clean only removes files it knows about.

@WalterBright
Copy link
Member Author

@rainers again, where is CWS_MAKEFILES? I updated my copy of druntime from master, I don't see it.

Also, this sort of error message is embarrassingly bad:

test/shared/Makefile:61:    rm -rf $(ROOT)
make: *** [checkwhitespace] Error 1

@rainers
Copy link
Member

rainers commented Jul 12, 2015

@rainers again, where is CWS_MAKEFILES? I updated my copy of druntime from master, I don't see it.

https://github.com/WalterBright/druntime/blob/d60cf660495a3ef33c3d607314d7aff159acba12/posix.mak#L210

Also, this sort of error message is embarrassingly bad:

This is the result of running grep. checkwhitespace is not available and cannot be compiled because the build server does not provide a D compiler (with phobos) when building druntime.

@rainers
Copy link
Member

rainers commented Jul 12, 2015

I never trust freshen, as sometimes files get removed from git.

You won't notice anyway because you don't have git installed on the system where you use it ;-)

@WalterBright
Copy link
Member Author

Oh, I thought CWS_MAKEFILES was supposed to be a file.

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

DmitryOlshansky added a commit that referenced this pull request Jul 12, 2015
[reg] fix Issue 14750 - druntime/test/coverage was added to druntime, but n…
@DmitryOlshansky DmitryOlshansky merged commit 6bc37c0 into dlang:master Jul 12, 2015
@WalterBright WalterBright deleted the fix14750 branch July 12, 2015 20:11
tramker pushed a commit to tramker/druntime that referenced this pull request Jan 27, 2016
[REG] fix Issue 14750 - druntime/test/coverage was added to druntime, but not to the MANIFEST - zip file broken again
Conflicts:
	posix.mak

Signed-off-by: Martin Krejcirik <mk@krej.cz>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants