Comments (38)
@akrantz the .bin
folder is created by NPM by symlinking whatever you specify in the bin
field in package.json
. You have specified lib/cli.js
, which is exactly what is symlinked into the .bin
folder. You have CRLF endings in this file, hence the issue here.
npm/npm#12371 seems to relate, but I don't think it was ever definitively resolved (despite being marked as such). I'm not sure it can be resolved to everyone's satisfaction, because NPM shouldn't really be assuming anything about people's uploaded code.
By adding the shebang #!/usr/bin/env node
to the top of this file, the authors are effectively attempting to create a *nix compatible executable file. They're not following the spec by then using CRLF endings in that file, which is why it is breaking—which is the authors' fault, not NPMs.
It is never going to be a node issue, because node doesn't come into the picture until the shell successfully parses the shebang, so node version is not relevant. It could be that previous versions of npm worked round this mistake, or it could even be that previous versions of bash on macOS could deal with CRLF, which might be why it seems to work for some.
from office-addin-scripts.
I also have the same issue - and can confirm it doesn't happen when using npm - only yarn.
yarn: 1.17.3
npm: 6.9 (working)
node: v10.16.3
currently I'm adding a postinstall script to our repo:
awk 'BEGIN{RS="^$";ORS="";getline;gsub("\r","");print>ARGV[1]}' ./node_modules/office-addin-debugging/cli.js
awk 'BEGIN{RS="^$";ORS="";getline;gsub("\r","");print>ARGV[1]}' ./node_modules/office-addin-debugging/lib/cli.js
But I think the true fix is as @gordonmleigh describes.
from office-addin-scripts.
I believe it is a yarn vs npm thing. NPM does appear to convert line endings when linking the bin files into .bin
. Yarn has an outstanding issue at yarnpkg/yarn#5480. It's still on package maintainers to publish the correct line endings though, regardless of whether one package manager works around it for you.
from office-addin-scripts.
I ran into this problem too. Fixed it by changing the line endings of node_modules/office-addin-debugging/cli.js
from office-addin-scripts.
I'm seeing the same issue with Node 10. Changing the line ending fixed it.
from office-addin-scripts.
I'm having the same issue and created a minimal example with Github Actions of this problem before landing here:
https://github.com/bockstaller/lineending_demo/actions/runs/1739321056
Is there any progress on this topic?
from office-addin-scripts.
The files in the bin folder are copied there according to this configuration option: https://docs.npmjs.com/cli/v8/configuring-npm/package-json#bin
This carries over the CRLF eol to the build output.
But I did find this option https://www.typescriptlang.org/tsconfig#newLine, which allows to normalize the build outputs lineendings, independent of plattform.
Testing it on my mac and windows machine showed that the build output, indeed changed from CRLF to LF with the option activated.
from office-addin-scripts.
So after coming back to this and re-thinking through the comments I think I understand things a bit better now. I was thinking about maintaining line ending in the files themselves as they were stored in github an it's influence on npm, but now I understand that it more about the build output itself. While the file contents influence that (as indicated by using prettier and some of the things I was looking at) I think the better option is the complier flag for typescript (which I didn't understand before) that makes the js file output us LF reguardless of with the original code files (i.e. working directory) have. I am make a PR the updates the tsconfig files to do this and then the next time we update the packages they should be corrected.
from office-addin-scripts.
We don't create the .bin folder. It is created by "npm install" based on the "bin" in package.json.
Please let us know what versions of Node and NPM you are using:
node --version
npm version
Also, on my Mac, I have installed Node via homebrew. It would help to know how you've installed Node and whether you are also using something like NVM to be able to switch NPM versions.
from office-addin-scripts.
Node: 12.6.0
Yarn: 1.17.3
from office-addin-scripts.
Please see if you have different results when using Node 10.
from office-addin-scripts.
node 8 and 10 both work correctly.
from office-addin-scripts.
@jpshankle Thanks for the info. I think this is a bug with Node 12, although I couldn't find any issues related to it when searching the repository. Do you want to open an issue there? https://github.com/nodejs/node/issues
Also, do all of the node_modules/.bin files have the issue or is it specific to a particular package?
from office-addin-scripts.
I have only run into it in the office-addin-debugging
package, so I'm not sure what other packages it might be happening in
from office-addin-scripts.
OK, perhaps this is just a git issue in that the line endings are not converted to LF only on check-in, and it may be due to this one particular package which is why we don't see it consistently across packages.
from office-addin-scripts.
@TCourtneyOwen I wonder if this is really a git issue or rather from the fact that we usually build and publish from Windows machines.
from office-addin-scripts.
Also, this mentions that .bin is symlinked to lib/cli.js
but now these should all be symlinked to cli.js
, and it requires(lib/cli.js
). I'm still not sure what the real issue is here because I've not had problems when using these packages on my mac with Node 10. I'm also curious if Node 12 becoming LTS will result in a different behavior.
from office-addin-scripts.
@akrantz Are you building these packages yourself on your Mac or are you installing from NPM?
from office-addin-scripts.
I think the project just needs a .gitattributes
file with say cli.js text eol=lf
from office-addin-scripts.
When I had investigated this months ago, I was not able to reproduce on my Mac when installing the packages from NPM. It seemed related to the Node version at the time based on information provided. Given the latest discussion, there seems to be the possibility that because the published packages have CRLF in the cli.js or lib/cli.js or other files, this is potentially an issue. If that is really the case though it seems this would be an issue for all of the these packages which are published when run on Mac, and that does not seem to be the case. So, I feel skeptical that this is really the issue but wanted to gather the data so that I can know if there are steps we could take to resolve this issue.
from office-addin-scripts.
I'm also seeing this issue on macOS with both yarn and npx. Any progress on this?
yarn: 1.22.11
npm/npx: 7.22.0
node: v16.8.0
from office-addin-scripts.
I am unfamiliar with some of the intricacies of npm and yarn in relating to the .bin folder and the line endings. We have code files cli.js in the root and the cli.ts in the src folder. The lib/cli.js is generated by the build and I'm not sure how the .bin files in node_modules folder relates to any these. Which code files would need to be modified to use LF in order for the right thing to happen on mac when using yarn (note I have not tried to reproduce this myself yet).
from office-addin-scripts.
Without trying this out myself, it should probably be enough to convert the cli.js
file itself from CRLF to LF. It seems to be one of the very few files that have CRLF EOL in the repo, e.g. the src/cli.ts
file you mentioned is already LF, as is the package.json
of this package.
Furthermore, the files probably should not be checked out with CRLF on Windows for building the packages for an NPM release. I.e. build them on a Linux or Mac machine or VM, in WSL, or set the Git repo to check out the files as they are in the repo (git config core.autocrlf false
).
from office-addin-scripts.
TBH this is new territory for me, but I got it running locally on my Mac
- Clone this [oas] and my minimal example [me] locally
- [me]
yarn install && yarn run validate
to verify that the problem persists - [oas] followed this guide (on the mac only the second half, because here I am running LF per default)
- [oas]
npm install
,cd office-addin-manifest && npm install
,npm run build && yarn link
- [me]
yarn link "office-addin-manifest" && yarn run validate
-> the problem no longer exists
So theoretically ™️ everything is fixed now. Assuming yarn uses the same logic everywhere and my mental model is somewhat correct.
I'll push the updated files in a minute
from office-addin-scripts.
So this is what I'm seeing and learning:
- VS Code shows the same file's line endings in the status bar differently on a Mac vs. a PC (mac can show LF while on windows the same file shows CRLF)
- A couple files in this repo show the same in both platforms (CRLF) and cli.js is one of those files
- There is a git command to show the line ending information: git ls-files --eol
- This git command shows an index ending and a working folder ending and the index ending doesn't change on the different plaforms while the working directory one does.
- cli.js shows "mixed" line endings for both working and index
I also found the command 'git add --renormalize .' will fix the index ends to be the standard LF that all the other files list. I think this is the correct change . . . but I'm not sure how to verify it before checking it in.
from office-addin-scripts.
You can validate a part of the fix like I did here bockstaller/lineending_demo@c3fca79?diff=split
I moved the build .tgz into my minimal example and install it manually, as described here. This fixes the problem in the yarn_new_package
-job, while it persists in the old yarn
-job where the new package isn't installed https://github.com/bockstaller/lineending_demo/actions/runs/1741951208
But this doesn't help you avoid regressions
from office-addin-scripts.
Git has a way to normalize line endings for the working tree, but I don't think this is related to the issue with line endings in .bin files. The working tree will have line endings as CRLF on Windows whereas it is LF on Mac/Linux. When checked in, they are normalized to LF. However, I don't think this issue relates to the .bin files.
It's likely either because the NPM packages are built on Windows and then published to NPM, so perhaps the .bin files which are published have CRLF because of that.
It would help to have more clarity about the root cause of the problem.
from office-addin-scripts.
I'd like to investigate more and see if the source files make a difference, but I need clarity around how .bin files are generated and used and why that seems related to the problem.
from office-addin-scripts.
I'm noticing that when I import office-adding-debugging to a project that on windows the file in the bin folder include 3 named office-addin-debugging, office-addin-debugging.cmd, and office-addin-debugging.ps1. The cmd file uses CRLF while the other two use LF. The code inside these files references the clit.js file in the root (which imports the one in the lib dir). But on mac it looks like a direct symlink to cli.js in the root. This is all based off using npm . . . but it seems like yarn is doing something different and I still need to try that out.
vbudovski-plexus said that changing the line endings in the root cli.js file fixed it for them and that is the file that git identified as mixed in both the index and working versions (using --renormalize updated the index case to LF).
from office-addin-scripts.
I installed and used yarn on my Mac machine and was unable to reproduce the error myself (deleted and had it create the node_modules folder and everything in it). The files in .bin looked the same as those generated by npm on both mac and winodows respectivly. Something else must be going on.
from office-addin-scripts.
I setup a fresh Ubuntu-20.04 WSL instance and installed node/npm with yo and generator-office. When generated a project it had various dependencies from Office-Addin-Scripts. Most of the .TS files in the various src folders (within node_modules) all where saved with CRLF. The files in node_modules/.bin are symlinks to their respective files.
As this is the first one I ran into issues with, I will use it as an example:
office-addin-lint: /home/ck/mj/node_modules/.bin/office-addin-lint
-> \home\ck\mj\node_modules\office-addin-lint\lib\cli.js
\home\ck\mj\node_modules\office-addin-lint\lib\cli.js
is saved as CRLF.
As a result whenever I try to execute npm run lint
I get the error /usr/bin/env: ‘node\r’: No such file or directory
If I where to guess (and a few other files have this set):
I would change any endOfLine prettier setting to "LF".
from office-addin-scripts.
I don't think the issue isn't what is checked out with Git (the "working tree") but what is being published to NPM. When NPM install is run, the question is where the contents of the .bin folders come from and why it's not normalized to LF.
from office-addin-scripts.
Just saw the previous comment about the tsconfig newLine setting. That sounds promising.
from office-addin-scripts.
I don't think the issue isn't what is checked out with Git (the "working tree") but what is being published to NPM. When NPM install is run, the question is where the contents of the .bin folders come from and why it's not normalized to LF.
The lint jobs are ran before the build. Since endOfLine is set to auto, it will preserve the EOL settings for the file it is processing. If you change the endOfLine setting to LF from auto, the files that are generated in the build process (thus the .bin folder) will then have LF EOF characters.
from office-addin-scripts.
If the .TS file is ever saved as CRLF and the prettier rules stay as auto, then the files will never be changed to LF. Example: If you look at office-addin-lint\src\cli.ts
it is saved as CRLF. As such, any file generated from it will be CRLF. Since prettier is set to auto for endOfLine, it will not change it to LF or even flag is as incorrect if npm run lint
or npm run lint:fix
is ran.
This will probably only be an issue if its built by someone who has core.autocrlf set to true. As on checkout: converts LF to CRLF. So when the user runs the install with that setting, they will get CRLF files. But if the package is built on a machine that has not changed core.autocrlf OR the build process runs lint:fix first (and the prettier files are chnged from auto to LF) then the resulting files will be correct.
from office-addin-scripts.
Building before changing endOfLine
in eslint-plugin-office-addins/src/main.ts and eslint-plugin-office-addins/.eslintrc
Building after changing endOfLine
to lf
Running lint:fix
after changing endOfLine
to lf
in the root folder will fix any files you need to as well.
from office-addin-scripts.
temp fix (expericned tis on fresh ubuntu machine):
- in node_modules/.bin:
cat office-addin-dev-certs | tr -d '\r' > office-addin-dev-certs
from office-addin-scripts.
This should be fixed in the current office-addin-dev-certs package (yo office output still needs to be updated).
from office-addin-scripts.
Related Issues (20)
- @microsoft/teams-manifest requires an insecure axios version HOT 12
- Web Addin certificate issue while building docker container image
- office-addin-manifest export fails when localization file is referenced
- getGraphToken is missing HOT 1
- why is the exception ignored when executing "certs verify" command? HOT 2
- office-addin-debugging Unsupported engine HOT 4
- CustomFunctionsMetadataPlugin ERROR in Conflict: Multiple assets emit different content to the same filename HOT 5
- validate works in CLI but office fails validation HOT 10
- "Icon URL Unreachable" error but URL is reachable (while validating manifest.xml) HOT 7
- custom-function-metadata should not use fs in parseTree HOT 3
- packages should specific exactly what they export. HOT 1
- Issues with AppContainer Loopback Logic in `office-addin-debugging` and `office-addin-dev-settings` HOT 4
- Should office-addin-dev-certs work for browsers? HOT 2
- Office-AddIn-Mock failed to mock getOfficeDisplayLanguage and build a project
- "Unable to start the dev server" on Ubuntu WSL HOT 2
- office-addin-sso needs to be updated to work with the JSON manifest HOT 1
- office-addin-manifest returns success on non-existent manifest file path HOT 3
- excel-custom-functions-js Unsupported engine (node 20) HOT 1
- shared run time add-in parameter problem HOT 6
- Azure Identity Libraries and Microsoft Authentication Library Elevation of Privilege Vulnerability HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from office-addin-scripts.