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

chore: do not add newlines to object data #1352

Merged
merged 1 commit into from
May 23, 2018

Conversation

achingbrain
Copy link
Member

For consistency with go-ipfs we should not add newlines to the output of ipfs object data.

go:

$ echo 'Hello' | ipfs files write --create /hello.txt
$ ipfs files ls / -l | grep hello.txt
hello.txt	QmcnK4wpoB6ozwZ7Siye8wyVTHYKNNoNvapPMLuCYUrHK2	6
$ ipfs object data QmcnK4wpoB6ozwZ7Siye8wyVTHYKNNoNvapPMLuCYUrHK2 | xxd
00000000: 0802 1806 2006                           .... .

js without this PR (with experimental mfs support) - note the extra 0a at the end:

$ echo 'Hello' | JSipfs files write --create /hello.txt
$ jsipfs files ls / -l | grep hello.txt
hello.txt	QmZo8WtosAnHDHXWCvih3xJsiENgdaKBmdx4gKnYmRtSUu	6 
$ jsipfs object data QmZo8WtosAnHDHXWCvih3xJsiENgdaKBmdx4gKnYmRtSUu | xxd
00000000: 0802 1806 2006 0a                        .... ..

js with this PR:

$ echo 'Hello' | JSipfs files write --create /hello.txt
$ jsipfs files ls / -l | grep hello.txt
hello.txt	QmZo8WtosAnHDHXWCvih3xJsiENgdaKBmdx4gKnYmRtSUu	6 
$ jsipfs object data QmZo8WtosAnHDHXWCvih3xJsiENgdaKBmdx4gKnYmRtSUu | xxd
00000000: 0802 1806 2006                           .... .

You may notice that the CIDs are different between go and js although the content is identical - this is because they structure the DAG differently for small files - js uses a single node if the data will fit in one node, go always creates one child that holds the data but this can be addressed in a separate PR.

@ghost ghost assigned achingbrain May 14, 2018
@ghost ghost added the status/in-progress In progress label May 14, 2018
@achingbrain achingbrain force-pushed the do-not-add-newlines-to-object-data branch from 718ba9b to c277ea0 Compare May 14, 2018 11:13
@@ -36,7 +36,7 @@ describe('object', () => runOnAndOff((thing) => {
})
})

it('put', () => {
it.only('put', () => {
Copy link
Member

Choose a reason for hiding this comment

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

remove .only before merging

@achingbrain achingbrain force-pushed the do-not-add-newlines-to-object-data branch from c277ea0 to b91dbcc Compare May 14, 2018 11:31
fsdiogo
fsdiogo previously approved these changes May 15, 2018
Copy link
Contributor

@fsdiogo fsdiogo left a comment

Choose a reason for hiding this comment

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

Good catch 👍

@daviddias
Copy link
Member

Please rebase master onto this branch for green CI

@alanshaw
Copy link
Member

@diasdavid can you please approve this PR so it can be merged 🙏

@daviddias
Copy link
Member

@alanshaw approved. Please add an API change to #1320

@alanshaw alanshaw merged commit 2e40fb8 into master May 23, 2018
@ghost ghost removed the status/in-progress In progress label May 23, 2018
@alanshaw alanshaw deleted the do-not-add-newlines-to-object-data branch May 23, 2018 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants