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

Please make the build reproducible. #145

Merged
merged 1 commit into from Nov 10, 2017

Conversation

Projects
None yet
2 participants
@lamby
Contributor

lamby commented Nov 7, 2017

Whilst working on the Reproducible Builds effort [0], we noticed
that nbsphinx could not be built reproducibly.

Patch attached that uses different Javascript to get the sibling
<div/> element instead of generating a (random) UUID and putting
that in the DOM.

[0] https://reproducible-builds.org/

Please make the build reproducible.
Whilst working on the Reproducible Builds effort [0], we noticed
that nbsphinx could not be built reproducibly.

Patch attached that uses different Javascript to get the sibling
<div/> element instead of generating a (random) UUID and putting
that in the DOM.

 [0] https://reproducible-builds.org/
@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 7, 2017

Contributor

This is being tracked in Debian at https://bugs.debian.org/881094

Contributor

lamby commented Nov 7, 2017

This is being tracked in Debian at https://bugs.debian.org/881094

@mgeier

This comment has been minimized.

Show comment
Hide comment
@mgeier

mgeier Nov 8, 2017

Member

Thanks!

I think I know now what you mean ... you want to include the generated HTML docs in the Debian package, and that's where the random UUID that's currently in there doesn't allow reproducible builds.
Right?

currentScript doesn't seem to be available for IE: https://developer.mozilla.org/en-US/docs/Web/API/Document/currentScript
Is anyone out there still using IE?

If yes, we could probably use the work-around suggested in https://stackoverflow.com/a/4172217/

Something like

var element = document.createElement("div");
// work-around for currentScript, see https://stackoverflow.com/a/4172217/
var scripts = document.getElementsByTagName("script");
var thisScript = scripts[scripts.length - 1];
thisScript.parentNode.insertBefore(element, thisScript);

This leaks the temporary variables into the following user-provided JavaScript code, so we should probably avoid that:

var element = document.createElement("div");
(function () {
    // work-around for currentScript, see https://stackoverflow.com/a/4172217/
    var scripts = document.getElementsByTagName("script");
    var thisScript = scripts[scripts.length - 1];
    thisScript.parentNode.insertBefore(element, thisScript);
})();

I know this is getting quite ugly ... what do you think about it?

And just out of curiosity: why do you need to use previousSibling twice?

Member

mgeier commented Nov 8, 2017

Thanks!

I think I know now what you mean ... you want to include the generated HTML docs in the Debian package, and that's where the random UUID that's currently in there doesn't allow reproducible builds.
Right?

currentScript doesn't seem to be available for IE: https://developer.mozilla.org/en-US/docs/Web/API/Document/currentScript
Is anyone out there still using IE?

If yes, we could probably use the work-around suggested in https://stackoverflow.com/a/4172217/

Something like

var element = document.createElement("div");
// work-around for currentScript, see https://stackoverflow.com/a/4172217/
var scripts = document.getElementsByTagName("script");
var thisScript = scripts[scripts.length - 1];
thisScript.parentNode.insertBefore(element, thisScript);

This leaks the temporary variables into the following user-provided JavaScript code, so we should probably avoid that:

var element = document.createElement("div");
(function () {
    // work-around for currentScript, see https://stackoverflow.com/a/4172217/
    var scripts = document.getElementsByTagName("script");
    var thisScript = scripts[scripts.length - 1];
    thisScript.parentNode.insertBefore(element, thisScript);
})();

I know this is getting quite ugly ... what do you think about it?

And just out of curiosity: why do you need to use previousSibling twice?

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 8, 2017

Contributor

you want to include the generated HTML docs in the Debian package, and that's where the random UUID that's currently in there doesn't allow reproducible builds.

Correct!

And just out of curiosity: why do you need to use previousSibling twice?

The first previousSibling matches the whitespace between the tags. :)

[..] I know this is getting quite ugly ... what do you think about it?

Well, I've seen worse. I think it's okay as it's automatically-generated code and we can add a lot of comments. Alternatively, we could just not support IE... *g*

Contributor

lamby commented Nov 8, 2017

you want to include the generated HTML docs in the Debian package, and that's where the random UUID that's currently in there doesn't allow reproducible builds.

Correct!

And just out of curiosity: why do you need to use previousSibling twice?

The first previousSibling matches the whitespace between the tags. :)

[..] I know this is getting quite ugly ... what do you think about it?

Well, I've seen worse. I think it's okay as it's automatically-generated code and we can add a lot of comments. Alternatively, we could just not support IE... *g*

@mgeier mgeier merged commit f808fc1 into spatialaudio:master Nov 10, 2017

@mgeier

This comment has been minimized.

Show comment
Hide comment
@mgeier

mgeier Nov 10, 2017

Member

OK, let's not care about IE.

If somebody complains, we can switch to the solution above.

Member

mgeier commented Nov 10, 2017

OK, let's not care about IE.

If somebody complains, we can switch to the solution above.

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 10, 2017

Contributor

let's not care about IE

Contributor

lamby commented Nov 10, 2017

let's not care about IE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment