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

dist manifest on windows creates a file with newlines #18

Closed
wchristian opened this issue Aug 30, 2011 · 17 comments

Comments

Projects
None yet
4 participants
@wchristian
Copy link
Member

commented Aug 30, 2011

Making a note here because rt is down again. Sometime in the future i need to poke at this to make sure it generates linux newlines all the time.

@ghost

This comment has been minimized.

Copy link

commented Aug 30, 2011

You can always email rt.cpan.org and the new ticket will get created
when the mail servers come back up.

-- David

On Tue, Aug 30, 2011 at 10:04 AM, wchristian
reply@reply.github.com
wrote:

Making a note here because rt is down again. Sometime in the future i need to poke at this to make sure it generates linux newlines all the time.

Reply to this email directly or view it on GitHub:
#18

@mohawk2

This comment has been minimized.

Copy link
Member

commented Jul 22, 2014

Is this still a live issue?

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Jul 23, 2014

Confirmed with 1.64, EU::Manifest still creates windows newlines in MANIFEST on windows.

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 3, 2014

@wchristian, since the issue is open on EU::M, any reason to leave this one open? If not, could we close it?

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2014

This bug is about a feature of EU::MM that is broken. Regardless of whether there is an issue on EU::M, this behavior is still broken, and unless the dependency is removed and replaced with properly working code, or a different dependency, or the dependency is fixed, the feature in EU::MM will continue to be broken. As such i cannot see any reason why this should be closed at all at this time.

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

I'll make a test with TODO if you can give me an idea of the behaviour we're talking about?

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2014

Sure:

Reproduction steps:

  1. Be on Windows with either ActivePerl or Strawberry Perl
  2. Generate Makefile via perl Makefile.PL
  3. Execute dmake manifest
  4. Grep MANIFEST for \r\n

Actual result:

\r\n is found

Expected result:

\r\n is not found

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

I've looked in EU::M's tests. The test with "SPLITTER" in it seems to indicate that EU::M expressly tolerates CRLF. Are we saying this is a thing EU::MM can't handle? If so, that seems like the problem, and I can have a go at that.

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2014

I don't know if anything can handle CRLFs in the MANIFEST file. I'd more importantly rather not find out, and also not have spurious differences in the tarball between releases only because the release OS was different.

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

That suggests the current EU::M test is wrong? @karenetheridge, your thoughts?

@karenetheridge

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

If the tests are allowing CRLFs to appear in the result, then the tests are wrong. As @wchristian says, we want the produced files to look the same no matter what architecture they were produced on.

(The same principle applies to EUMM producing Makefiles, and any other tool that generates or appends to files.)

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

That seems fair. I'm close to a fix on this.

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 4, 2014

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 6, 2014

@wchristian, sorry if I'm asking the same question as previously, but: EU::Man will soon cease to produce CRLF. Putting aside aesthetic issues of dists differing if made on different architectures, is there a functional problem caused to EUMM if CRLFs occur? If not, is this truly a remaining EUMM issue? And if not that, can we close this one?

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2014

@mohawk2 Summary from the IRC discussion: Until there's an actual fix for this on CPAN, i do not think this should be closed.

Notably i also think that when a fix of EU::M is released, the version number in the EU::MM dependencies should be bumped and a new EU::MM release made. This is how it is handled for the vast majority of modules on CPAN. I do realize EU::MM is slightly different, so the process might vary, and i do of course not have the ability to give instructions, only describe how i wish for this to be handled.

@mohawk2

This comment has been minimized.

Copy link
Member

commented Aug 9, 2014

New EU::M 1.65 released and bundled in EUMM in 0ce91f3. Pretty sure this issue can be considered closable ;-)

@wchristian

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2014

This issue is done. :)

@wchristian wchristian closed this Aug 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.