Setting autocrlf and safecrlf on workers

For context, see nodejs/reliability#12 and the minutes of the last build meeting #1448 It was agreed to fix those workers by configuring git to not fix the line breaks.

I'll fix the nodes manually by running

git config --global core.autocrlf false
git config --global core.safecrlf false

on nodes with trouble checking out files with CRLF.

Feel free to add nodes in the following list if you find a new worker with this issue or have fixed a worker

Maybe we should just put the configuration in https://github.com/nodejs/build/blob/master/jenkins/scripts/node-test-commit-pre.sh instead?

+1 for configuration changes in scripts rather than manually done.

Also, as far as I can tell the containers have their configs set in the Jenkins instead of using the script

@joyeecheung

autocrlf should not be changed in the Windows machines, we want to ensure new contributors can compile with a default installation of Git (we already ask for the Unix tools, but the less of those little details the better and some people might not want to change it). I understand this does not include Windows hosts for now, but there is support to run the V8 tests on Windows and the ChakraCore nightlies seem to be failing because of this.

@joyeecheung can we instead use https://github.com/nodejs/node/blob/master/.gitattributes to specify that that file is always CRLF? This is already done for vcbuild.bat.

@joaocgreis

@joaocgreis The failures in CI are caused by incorrect checkouts (not a fresh clone of nodejs/node), I don't see changing settings on our CI makes a difference since that's not how new contributors set up their environment anyway? For instance, developers don't see the modified: deps/v8/third_party/jinja2/LICENSE error, but we do.

can we instead use https://github.com/nodejs/node/blob/master/.gitattributes to specify that that file is always CRLF? This is already done for vcbuild.bat.

Does that work with https://github.com/nodejs/node/blob/8a41470c85536119a6dbeaf4b40dc9ebc4c453c4/deps/v8/.gitattributes#L1-L2 ? It's a file in deps/v8 but not actually in V8's upstream source, and V8 actually uses LF for all its own text files in its own .gitattributes

I don't see changing settings on our CI makes a difference since that's not how new contributors set up their environment anyway?

It should be, Windows CI machines are set up in the same way that we recommend contributors to set their own (with a few additions for Jenkins). Using a different autocrlf setting in CI can cause tests to fail because of different line endings.

Please let me take a better look at this, I'd much rather find a solution that doesn't involve changing machine configuration.

As it's a file maintained by us and not in V8's source, maybe we can just patch it to use LF? That way it won't conflict with deps/v8/.gitattributes

@targos

As it's a file maintained by us and not in V8's source, maybe we can just patch it to use LF?

That sounds good to me, if @targos is ok with it.

@joyeecheung

I confirmed this is only a problem depending on the Git version. Happens with older versions (tested 1.9.1) but not with newer (2.18.0). nodejs/node#22340 (thanks for opening @joyeecheung!) should fix the problem, but in any case I'd be ok with changing the crlf settings only in machines where Git can't be updated.

we want to ensure new contributors can compile with a default installation of Git

That was my motivation as well. Setting git to not touch the files in any way would break Windows build in case file are improperly checked in (e.g. .bat file with just LF). So we will be actually testing if the code can compile even from tarball.

So we will be actually testing if the code can compile even from tarball.

Given that we have actually broken this in the past this would be better off as a separate job (that actually unpacks the tarball and attempts to compile/test it).

deps: fix CRLF in text file not present in upstream

I run this groovy script:

import hudson.util.RemotingDiagnostics
import jenkins.model.Jenkins
 
String agent_name = 'test-digitalocean-ubuntu1604_sharedlibs_container-x64-3'
//groovy script you want executed on an agent
String groovy_script = '''
println "git config --global core.autocrlf false".execute().text
println "git config --global core.safecrlf false".execute().text
println "git config --global -l".execute().text
'''.trim()
 
Jenkins.instance.slaves.findAll { agent ->
    agent.name ==~ /(?i).*container.*/
}.forEach { agent ->
    println "################## ${agent.name} #################"
    println  RemotingDiagnostics.executeGroovy(groovy_script, agent.channel)
    println '####################'
}

to get:

################## test-digitalocean-alpine37_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-alpine37_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-alpine38_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-alpine38_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-10 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-3 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-4 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-5 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-6 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-7 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-8 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1604_sharedlibs_container-x64-9 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1804_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-digitalocean-ubuntu1804_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-alpine37_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-alpine38_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1604_sharedlibs_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1604_sharedlibs_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1604_sharedlibs_container-x64-3 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1604_sharedlibs_container-x64-4 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1604_sharedlibs_container-x64-5 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-joyent-ubuntu1804_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-alpine37_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-alpine38_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1604_sharedlibs_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1604_sharedlibs_container-x64-2 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1604_sharedlibs_container-x64-3 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1604_sharedlibs_container-x64-4 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1604_sharedlibs_container-x64-5 #################
core.autocrlf=false
core.safecrlf=false


####################
################## test-softlayer-ubuntu1804_container-x64-1 #################
core.autocrlf=false
core.safecrlf=false


####################

Also for good measure run with agent.name ==~ /(?i).*linuxone.*/:

################## test-linuxonecc-rhel72-s390x-1 #################


core.autocrlf=false
core.safecrlf=false


####################
################## test-linuxonecc-rhel72-s390x-2 #################


core.autocrlf=false
core.safecrlf=false


####################
################## test-linuxonecc-rhel72-s390x-3 #################


core.autocrlf=false
core.safecrlf=false


####################

deps: fix CRLF in text file not present in upstream

Help, this can't be killed:

[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$ git status
# HEAD detached at 4b7cd4b
# Changes not staged for commit:
#   (use "git add/rm <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       deleted:    deps/v8/third_party/jinja2/LICENSE
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       build/
no changes added to commit (use "git add" and/or "git commit -a")
[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$ git checkout deps/v8/third_party/jinja2/LICENSE
[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$ git status
# HEAD detached at 4b7cd4b
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       modified:   deps/v8/third_party/jinja2/LICENSE
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       build/
no changes added to commit (use "git add" and/or "git commit -a")
[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$ git --version
git version 1.8.3.1
[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$ git config --global -l
core.autocrlf=false
core.safecrlf=false
core.eol=lf
[iojs@test-linuxonecc-rhel72-s390x-3 rhel72-s390x]$

So apparently with git 1.8 we need to delete the workspace for the settings to work.

I thought the errors would not appear if you are checking out from a commit without the CRLF file and not rebasing from somewhere that contains it? (but yeah, it'll appear the next time we have a file like that in deps/v8)

I thought the errors would not appear if you are checking out from a commit without the CRLF file and not rebasing from somewhere that contains it? (but yeah, it'll appear the next time we have a file like that in deps/v8)

I was testing explicitly with "bad" commits:

GIT_REMOTE_REF	4b7cd4bd60623b9c69d791112f4500575a3e9e7d
REBASE_ONTO		2ce03804a6a2e55a034814ca40fafac654321e33

https://ci.nodejs.org/job/node-test-commit-linuxone/4190/

When everything is setup right, it passes.

deps: fix CRLF in text file not present in upstream

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

Navigation Menu

Repository navigation

Setting `autocrlf` and `safecrlf` on workers #1453

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Setting autocrlf and safecrlf on workers #1453

Description

Activity

joyeecheung commented on Aug 15, 2018

richardlau commented on Aug 15, 2018

joyeecheung commented on Aug 15, 2018

joaocgreis commented on Aug 15, 2018

joyeecheung commented on Aug 15, 2018

joyeecheung commented on Aug 15, 2018

joaocgreis commented on Aug 15, 2018

joyeecheung commented on Aug 15, 2018

joaocgreis commented on Aug 15, 2018

joaocgreis commented on Aug 15, 2018

refack commented on Aug 15, 2018

richardlau commented on Aug 15, 2018

2 remaining items

refack commented on Aug 18, 2018

refack commented on Aug 18, 2018

refack commented on Aug 20, 2018

refack commented on Aug 20, 2018

joyeecheung commented on Aug 20, 2018

refack commented on Aug 20, 2018

github-actions commented on Mar 8, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Setting `autocrlf` and `safecrlf` on workers #1453